[FFmpeg-devel] [PATCH] avformat/matroskadec: use mastering display metadata's own alloc function

James Almer jamrial at gmail.com
Fri Dec 2 21:46:12 EET 2016


On 12/2/2016 4:43 PM, wm4 wrote:
> On Fri,  2 Dec 2016 16:09:25 -0300
> James Almer <jamrial at gmail.com> wrote:
> 
>> Signed-off-by: James Almer <jamrial at gmail.com>
>> ---
>>  libavformat/matroskadec.c | 14 +++++++++-----
>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 017a533..7b070ff 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -1841,14 +1841,11 @@ static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
>>          // Use similar rationals as other standards.
>>          const int chroma_den = 50000;
>>          const int luma_den = 10000;
>> -        AVMasteringDisplayMetadata *metadata =
>> -            (AVMasteringDisplayMetadata*) av_stream_new_side_data(
>> -                st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
>> -                sizeof(AVMasteringDisplayMetadata));
>> +        int ret;
>> +        AVMasteringDisplayMetadata *metadata = av_mastering_display_metadata_alloc();
>>          if (!metadata) {
>>              return AVERROR(ENOMEM);
>>          }
>> -        memset(metadata, 0, sizeof(AVMasteringDisplayMetadata));
> 
> ok...
> 
>>          if (has_mastering_primaries) {
>>              metadata->display_primaries[0][0] = av_make_q(
>>                  round(mastering_meta->r_x * chroma_den), chroma_den);
>> @@ -1875,6 +1872,13 @@ static int mkv_parse_video_color(AVStream *st, const MatroskaTrack *track) {
>>                  round(mastering_meta->min_luminance * luma_den), luma_den);
>>              metadata->has_luminance = 1;
>>          }
>> +
>> +        ret = av_stream_add_side_data(st, AV_PKT_DATA_MASTERING_DISPLAY_METADATA,
>> +                                      (uint8_t *)metadata, sizeof(AVMasteringDisplayMetadata));
>> +        if (ret < 0) {
>> +            av_freep(&metadata);
>> +            return ret;
>> +        }
>>      }
>>      return 0;
>>  }
> 
> Uh what? Am I missing something, or do you rely on
> sizeof(AVMasteringDisplayMetadata) being part of the ABI? Because the
> alloc function only exists because the struct size is not part of the
> ABI.

I know, i mentioned as much in another email. Notice how sizeof() is
already being used above, in the code I'm removing.

This is mostly cosmetic, until either the size of the struct is defined
as public or a new function that returns said size is introduced.



More information about the ffmpeg-devel mailing list