Hi Thomas,
> On Nov 11, 2019, at 3:17 PM, Tomas Härdin <tjop...@acc.umu.se> wrote:
>
> lör 2019-11-09 klockan 15:46 -0800 skrev Baptiste Coudurier:
>> +static const UID mxf_dv_container_uls[] = {
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x01,0x01
>> }, // IEC DV25 525/60
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x02,0x01
>> }, // IEC DV25 626/50
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x40,0x01
>> }, // DV25 525/60
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x41,0x01
>> }, // DV25 625/50
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x50,0x01
>> }, // DV50 525/60
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x51,0x01
>> }, // DV50 625/50
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x60,0x01
>> }, // DV100 1080/60
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x61,0x01
>> }, // DV100 1080/50
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x62,0x01
>> }, // DV100 720/60
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x63,0x01
>> }, // DV100 720/50
>> +};
>> +
>> +static const UID mxf_dv_codec_uls[] = {
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x01,0x01,0x00
>> },
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x01,0x02,0x00
>> },
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x02,0x01,0x00
>> },
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x02,0x02,0x00
>> },
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x02,0x03,0x00
>> },
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x02,0x04,0x00
>> },
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x02,0x05,0x00
>> },
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x02,0x06,0x00
>> },
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x02,0x07,0x00
>> },
>> + {
>> 0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x02,0x08,0x00
>> },
>> +};
>
> Maybe a struct to keep these two together, or a comment saying they
> correspond to eachother 1:1
>
>> static int mxf_parse_dv_frame(AVFormatContext *s, AVStream *st, AVPacket
>> *pkt)
>> {
>> MXFContext *mxf = s->priv_data;
>> MXFStreamContext *sc = st->priv_data;
>> uint8_t *vs_pack, *vsc_pack;
>> + int apt = pkt->data[4] & 0x7;
>
> Can pkt->data ever be less than 5 bytes?
>
>> @@ -2182,28 +2144,29 @@ static int mxf_parse_dv_frame(AVFormatContext *s,
>> AVStream *st, AVPacket *pkt)
>>
>> switch (stype) {
>> case 0x18: // DV100 720p
>> - ul_index = INDEX_DV100_720_50 + pal;
>> + ul_index = 8+pal;
>
> Not a huge fan of magic constants. Maybe just move the INDEX_DV* enums
> to their own enum? Could use array index initializers to enforce that
> they're indeed in the right spots in mxf_dv_*_uls[]. Combined with the
> struct suggestion:
>
> static const struct {
> UID container_ul, codec_ul;
> } dv_uls[] = {
> [INDEX_DV25_525_60] =
> {{0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x0D,0x01,0x03,0x01,0x02,0x02,0x01,0x01},
>
> {0x06,0x0E,0x2B,0x34,0x04,0x01,0x01,0x01,0x04,0x01,0x02,0x02,0x02,0x01,0x01,0x00}},
> //...
> };
That would defeat the purpose of the patch, getting rid of the useless
constants :)
—
Baptiste
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
