Re: [FFmpeg-devel] [PATCH v2 4/7] avformat/matroska: add a few more Block Addition ID Type enum values

From: James Almer <jamrial@gmail.com>
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v2 4/7] avformat/matroska: add a few more Block Addition ID Type enum values
Date: Wed, 29 Mar 2023 21:48:50 -0300
Message-ID: <fd1c511d-1637-663e-483a-dabdf3d502cc@gmail.com> (raw)
In-Reply-To: <GV1P250MB073742DACD73D06B71DC8CDE8F8E9@GV1P250MB0737.EURP250.PROD.OUTLOOK.COM>

On 3/29/2023 9:43 PM, Andreas Rheinhardt wrote:
> James Almer:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavformat/matroska.h    | 4 ++++
>>   libavformat/matroskadec.c | 6 +++---
>>   libavformat/matroskaenc.c | 6 +++---
>>   3 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/libavformat/matroska.h b/libavformat/matroska.h
>> index b39517709c..3827675777 100644
>> --- a/libavformat/matroska.h
>> +++ b/libavformat/matroska.h
>> @@ -360,9 +360,13 @@ typedef enum {
>>   
>>   typedef enum {
>>     MATROSKA_BLOCK_ADD_ID_TYPE_DEFAULT                = 0,
>> +  MATROSKA_BLOCK_ADD_ID_TYPE_OPAQUE_DATA            = 1,
>>     MATROSKA_BLOCK_ADD_ID_TYPE_ITU_T_T35              = 4,
>> +  MATROSKA_BLOCK_ADD_ID_TYPE_DVCC                   = 0x64766343,
>> +  MATROSKA_BLOCK_ADD_ID_TYPE_DVVC                   = 0x64767643,
> 
> I regard the MKBETAG version as more enlightening, even though this is
> more in line with the spec.

I can add the MKBETAG version as a comment next on each line.

> 
>>   } MatroskaBlockAddIDType;
>>   
>> +#define MATROSKA_BLOCK_ADD_ID_OPAQUE_DATA 1
> 
> That's quite a lot to type.

Will rename to MATROSKA_BLOCK_ADD_ID_OPAQUE.

> 
>>   #define MATROSKA_BLOCK_ADD_ID_ITU_T_T35 4
>>   
>>   /*
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index ad8b352f31..b18c9e31dc 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -767,7 +767,7 @@ static EbmlSyntax matroska_segments[] = {
>>   };
>>   
>>   static EbmlSyntax matroska_blockmore[] = {
>> -    { MATROSKA_ID_BLOCKADDID,      EBML_UINT, 0, 0, offsetof(MatroskaBlockMore,additional_id), { .u = 1 } },
>> +    { MATROSKA_ID_BLOCKADDID,      EBML_UINT, 0, 0, offsetof(MatroskaBlockMore,additional_id), { .u = MATROSKA_BLOCK_ADD_ID_OPAQUE_DATA } },
>>       { MATROSKA_ID_BLOCKADDITIONAL, EBML_BIN,  0, 0, offsetof(MatroskaBlockMore,additional) },
>>       CHILD_OF(matroska_blockadditions)
>>   };
>> @@ -2404,8 +2404,8 @@ static int mkv_parse_block_addition_mappings(AVFormatContext *s, AVStream *st, M
>>               }
>>               track->blockaddid_itu_t_t35 = 1;
>>               break;
>> -        case MKBETAG('d','v','c','C'):
>> -        case MKBETAG('d','v','v','C'):
>> +        case MATROSKA_BLOCK_ADD_ID_TYPE_DVCC:
>> +        case MATROSKA_BLOCK_ADD_ID_TYPE_DVVC:
>>               if ((ret = mkv_parse_dvcc_dvvc(s, st, track, &mapping->extradata)) < 0)
>>                   return ret;
>>   
>> diff --git a/libavformat/matroskaenc.c b/libavformat/matroskaenc.c
>> index 954b7d828f..c083f55319 100644
>> --- a/libavformat/matroskaenc.c
>> +++ b/libavformat/matroskaenc.c
>> @@ -1612,9 +1612,9 @@ static void mkv_write_dovi(AVFormatContext *s, AVIOContext *pb, AVStream *st)
>>                                   + (2 + 1 + 4) + (2 + 1 + ISOM_DVCC_DVVC_SIZE);
>>   
>>           if (dovi->dv_profile > 7) {
>> -            type = MKBETAG('d', 'v', 'v', 'C');
>> +            type = MATROSKA_BLOCK_ADD_ID_TYPE_DVVC;
>>           } else {
>> -            type = MKBETAG('d', 'v', 'c', 'C');
>> +            type = MATROSKA_BLOCK_ADD_ID_TYPE_DVCC;
>>           }
>>   
>>           ff_isom_put_dvcc_dvvc(s, buf, dovi);
>> @@ -2657,7 +2657,7 @@ static int mkv_write_block(void *logctx, MatroskaMuxContext *mkv,
>>                                           &side_data_size);
>>       if (side_data && side_data_size >= 8 &&
>>           // Only the Codec-specific BlockMore (id == 1) is currently supported.
>> -        (additional_id = AV_RB64(side_data)) == 1) {
>> +        (additional_id = AV_RB64(side_data)) == MATROSKA_BLOCK_ADD_ID_TYPE_OPAQUE_DATA) {
>>           ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKADDITIONS);
>>           ebml_writer_open_master(&writer, MATROSKA_ID_BLOCKMORE);
>>           /* Until dbc50f8a our demuxer used a wrong default value
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".