[vlc-devel] [PATCH] vlc_es: use union in es_format_t

[Steve Lhomme]

On Wed, Jul 5, 2017 at 11:00 AM, Steve Lhomme <robux4 at gmail.com> wrote:
> This patch broke HDR and 360 metadata passing to the vout.

For 360 videos it seems that's because the MP4 demuxer initialize ES
to UNKNOWN_ES by default (as many other demuxers) and thus doesn't set
the default field of view properly.

For HDR it's because lavc_UpdateVideoFormat() modifies both the
"video" and "subs" part of the es_format_t. Overriding the transfer
function read from the source file...

> I'm investigating why.
>
> On Sun, May 21, 2017 at 8:30 PM, Jean-Baptiste Kempf <jb at videolan.org> wrote:
>> LGTM
>>
>> On Sun, 21 May 2017, at 20:19, Rémi Denis-Courmont wrote:
>>> This saves about 200 bytes per instance.
>>> ---
>>>  include/vlc_es.h     | 12 +++++++----
>>>  src/misc/es_format.c | 60
>>>  +++++++++++++++++++++++++++++++++-------------------
>>>  2 files changed, 46 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/include/vlc_es.h b/include/vlc_es.h
>>> index 1d76dac3df..bff3562d4e 100644
>>> --- a/include/vlc_es.h
>>> +++ b/include/vlc_es.h
>>> @@ -574,10 +574,14 @@ struct es_format_t
>>>      unsigned        i_extra_languages;    /**< length in bytes of extra
>>>      language data pointer */
>>>      extra_languages_t *p_extra_languages; /**< extra language data
>>>      needed by some decoders */
>>>
>>> -    audio_format_t  audio;    /**< description of audio format */
>>> -    audio_replay_gain_t audio_replay_gain; /*< audio replay gain
>>> information */
>>> -    video_format_t video;     /**< description of video format */
>>> -    subs_format_t  subs;      /**< description of subtitle format */
>>> +    union {
>>> +        struct {
>>> +            audio_format_t  audio;    /**< description of audio format
>>> */
>>> +            audio_replay_gain_t audio_replay_gain; /*< audio replay gain
>>> information */
>>> +        };
>>> +        video_format_t video;     /**< description of video format */
>>> +        subs_format_t  subs;      /**< description of subtitle format */
>>> +    };
>>>
>>>      unsigned int   i_bitrate; /**< bitrate of this ES */
>>>      int      i_profile;       /**< codec specific information (like real
>>>      audio flavor, mpeg audio layer, h264 profile ...) */
>>> diff --git a/src/misc/es_format.c b/src/misc/es_format.c
>>> index ab27a32ae8..80b4c914ab 100644
>>> --- a/src/misc/es_format.c
>>> +++ b/src/misc/es_format.c
>>> @@ -449,10 +449,20 @@ void es_format_Init( es_format_t *fmt,
>>>      fmt->i_extra_languages      = 0;
>>>      fmt->p_extra_languages      = NULL;
>>>
>>> -    memset( &fmt->audio, 0, sizeof(audio_format_t) );
>>> -    memset( &fmt->audio_replay_gain, 0, sizeof(audio_replay_gain_t) );
>>> -    video_format_Init( &fmt->video, 0 );
>>> -    memset( &fmt->subs, 0, sizeof(subs_format_t) );
>>> +    switch (fmt->i_cat)
>>> +    {
>>> +        case AUDIO_ES:
>>> +            memset(&fmt->audio, 0, sizeof (fmt->audio));
>>> +            memset(&fmt->audio_replay_gain, 0,
>>> +                   sizeof (fmt->audio_replay_gain));
>>> +            break;
>>> +        case VIDEO_ES:
>>> +            video_format_Init(&fmt->video, 0);
>>> +            break;
>>> +        case SPU_ES:
>>> +            memset(&fmt->subs, 0, sizeof (fmt->subs));
>>> +            break;
>>> +    }
>>>
>>>      fmt->b_packetized           = true;
>>>      fmt->i_bitrate              = 0;
>>> @@ -499,23 +509,25 @@ int es_format_Copy(es_format_t *restrict dst, const
>>> es_format_t *src)
>>>          }
>>>      }
>>>
>>> -    if (src->subs.psz_encoding != NULL)
>>> -    {
>>> -        dst->subs.psz_encoding = strdup(src->subs.psz_encoding);
>>> -        if (unlikely(dst->subs.psz_encoding == NULL))
>>> -            ret = VLC_ENOMEM;
>>> -    }
>>> -    if (src->subs.p_style != NULL)
>>> +    if (src->i_cat == VIDEO_ES)
>>> +        ret = video_format_Copy( &dst->video, &src->video );
>>> +
>>> +    if (src->i_cat == SPU_ES)
>>>      {
>>> -        dst->subs.p_style = text_style_Duplicate(src->subs.p_style);
>>> -        if (unlikely(dst->subs.p_style == NULL))
>>> -            ret = VLC_ENOMEM;
>>> +        if (src->subs.psz_encoding != NULL)
>>> +        {
>>> +            dst->subs.psz_encoding = strdup(src->subs.psz_encoding);
>>> +            if (unlikely(dst->subs.psz_encoding == NULL))
>>> +                ret = VLC_ENOMEM;
>>> +        }
>>> +        if (src->subs.p_style != NULL)
>>> +        {
>>> +            dst->subs.p_style = text_style_Duplicate(src->subs.p_style);
>>> +            if (unlikely(dst->subs.p_style == NULL))
>>> +                ret = VLC_ENOMEM;
>>> +        }
>>>      }
>>>
>>> -    int err = video_format_Copy( &dst->video, &src->video );
>>> -    if ( err != VLC_SUCCESS )
>>> -        return err;
>>> -
>>>      if (src->i_extra_languages > 0)
>>>      {
>>>          assert(src->p_extra_languages != NULL);
>>> @@ -548,11 +560,15 @@ void es_format_Clean(es_format_t *fmt)
>>>      assert(fmt->i_extra == 0 || fmt->p_extra != NULL);
>>>      free(fmt->p_extra);
>>>
>>> -    video_format_Clean( &fmt->video );
>>> -    free(fmt->subs.psz_encoding);
>>> +    if (fmt->i_cat == VIDEO_ES)
>>> +        video_format_Clean( &fmt->video );
>>> +    if (fmt->i_cat == SPU_ES)
>>> +    {
>>> +        free(fmt->subs.psz_encoding);
>>>
>>> -    if (fmt->subs.p_style != NULL)
>>> -        text_style_Delete(fmt->subs.p_style);
>>> +        if (fmt->subs.p_style != NULL)
>>> +            text_style_Delete(fmt->subs.p_style);
>>> +    }
>>>
>>>      for (unsigned i = 0; i < fmt->i_extra_languages; i++)
>>>      {
>>> --
>>> 2.11.0
>>>
>>> _______________________________________________
>>> vlc-devel mailing list
>>> To unsubscribe or modify your subscription options:
>>> https://mailman.videolan.org/listinfo/vlc-devel
>>
>>
>> --
>> Jean-Baptiste Kempf -  President
>> +33 672 704 734
>> _______________________________________________
>> vlc-devel mailing list
>> To unsubscribe or modify your subscription options:
>> https://mailman.videolan.org/listinfo/vlc-devel