Message ID | 1478565002-134460-1-git-send-email-zhennihuang@google.com |
---|---|
State | Superseded |
Headers | show |
On Mon, Nov 7, 2016 at 4:30 PM, Zhenni Huang <zhennihuang@google.com> wrote: > Also updates fate ref for rgb24-mkv as the output video will contain the > new metadata field and have different md5sum and file size. > --- > libavformat/mov.c | 25 +++++++++++++++++++++++-- > tests/ref/fate/rgb24-mkv | 4 ++-- > 2 files changed, 25 insertions(+), 4 deletions(-) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 4222088..2cb041f 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -1824,6 +1824,8 @@ static void mov_parse_stsd_video(MOVContext *c, > AVIOContext *pb, > uint8_t codec_name[32]; > int64_t stsd_start; > unsigned int len; > + int video_vendor_id = 0; > + char video_vendor_id_buffer[5]; > > /* The first 16 bytes of the video sample description are already > * read in ff_mov_read_stsd_entries() */ > @@ -1831,10 +1833,18 @@ static void mov_parse_stsd_video(MOVContext *c, > AVIOContext *pb, > > avio_rb16(pb); /* version */ > avio_rb16(pb); /* revision level */ > - avio_rb32(pb); /* vendor */ > + video_vendor_id = avio_rb32(pb); /* vendor */ > avio_rb32(pb); /* temporal quality */ > avio_rb32(pb); /* spatial quality */ > > + /* set video_vendor_id */ > + video_vendor_id_buffer[0] = (video_vendor_id >> 24) & 0xff; > + video_vendor_id_buffer[1] = (video_vendor_id >> 16) & 0xff; > + video_vendor_id_buffer[2] = (video_vendor_id >> 8) & 0xff; > + video_vendor_id_buffer[3] = (video_vendor_id >> 0) & 0xff; > + video_vendor_id_buffer[4] = 0; > + av_dict_set(&st->metadata, "vendor_id", video_vendor_id_buffer, 0); > + > st->codecpar->width = avio_rb16(pb); /* width */ > st->codecpar->height = avio_rb16(pb); /* height */ > > @@ -1880,9 +1890,20 @@ static void mov_parse_stsd_audio(MOVContext *c, > AVIOContext *pb, > int bits_per_sample, flags; > uint16_t version = avio_rb16(pb); > AVDictionaryEntry *compatible_brands = av_dict_get(c->fc->metadata, > "compatible_brands", NULL, AV_DICT_MATCH_CASE); > + int audio_vendor_id = 0; > + char audio_vendor_id_buffer[5]; > > avio_rb16(pb); /* revision level */ > - avio_rb32(pb); /* vendor */ > + audio_vendor_id = avio_rb32(pb); /* vendor */ > + > + /* set audio_vendor_id */ > + audio_vendor_id_buffer[0] = (audio_vendor_id >> 24) & 0xff; > + audio_vendor_id_buffer[1] = (audio_vendor_id >> 16) & 0xff; > + audio_vendor_id_buffer[2] = (audio_vendor_id >> 8) & 0xff; > + audio_vendor_id_buffer[3] = (audio_vendor_id >> 0) & 0xff; > + audio_vendor_id_buffer[4] = 0; > + > + av_dict_set(&st->metadata, "vendor_id", audio_vendor_id_buffer, 0); > > st->codecpar->channels = avio_rb16(pb); /* channel count > */ > st->codecpar->bits_per_coded_sample = avio_rb16(pb); /* sample size > */ > diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv > index 88d22c1..ba6311d 100644 > --- a/tests/ref/fate/rgb24-mkv > +++ b/tests/ref/fate/rgb24-mkv > @@ -1,5 +1,5 @@ > -94cce0d7d5b14b4c86e74a1ca454c5aa *tests/data/fate/rgb24-mkv.matroska > -58361 tests/data/fate/rgb24-mkv.matroska > +29e4fffecb2002912fc05ed910679ce3 *tests/data/fate/rgb24-mkv.matroska > +58390 tests/data/fate/rgb24-mkv.matroska > #tb 0: 1/10 > #media_type 0: video > #codec_id 0: rawvideo > -- > 2.8.0.rc3.226.g39d4020 > > I am really sorry about the FATE failure. Thanks for pointing it out.
On Mon, Nov 07, 2016 at 16:30:02 -0800, Zhenni Huang wrote: > - avio_rb32(pb); /* vendor */ > + video_vendor_id = avio_rb32(pb); /* vendor */ > avio_rb32(pb); /* temporal quality */ > avio_rb32(pb); /* spatial quality */ > > + /* set video_vendor_id */ > + video_vendor_id_buffer[0] = (video_vendor_id >> 24) & 0xff; > + video_vendor_id_buffer[1] = (video_vendor_id >> 16) & 0xff; > + video_vendor_id_buffer[2] = (video_vendor_id >> 8) & 0xff; > + video_vendor_id_buffer[3] = (video_vendor_id >> 0) & 0xff; > + video_vendor_id_buffer[4] = 0; > + av_dict_set(&st->metadata, "vendor_id", video_vendor_id_buffer, 0); Wouldn't you avoid shifting the bytes around if you did an avio_rl32() in the first place, i.e. video_vendor_id = avio_rl32(pb); Then you could just initialize the buffer to 0 and memcopy the id over. Or am I missing something? (If not: Others tend to complain about useless masks and useless shifts of 0. I personally think they're okay for readability.) Moritz
diff --git a/libavformat/mov.c b/libavformat/mov.c index 4222088..2cb041f 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -1824,6 +1824,8 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, uint8_t codec_name[32]; int64_t stsd_start; unsigned int len; + int video_vendor_id = 0; + char video_vendor_id_buffer[5]; /* The first 16 bytes of the video sample description are already * read in ff_mov_read_stsd_entries() */ @@ -1831,10 +1833,18 @@ static void mov_parse_stsd_video(MOVContext *c, AVIOContext *pb, avio_rb16(pb); /* version */ avio_rb16(pb); /* revision level */ - avio_rb32(pb); /* vendor */ + video_vendor_id = avio_rb32(pb); /* vendor */ avio_rb32(pb); /* temporal quality */ avio_rb32(pb); /* spatial quality */ + /* set video_vendor_id */ + video_vendor_id_buffer[0] = (video_vendor_id >> 24) & 0xff; + video_vendor_id_buffer[1] = (video_vendor_id >> 16) & 0xff; + video_vendor_id_buffer[2] = (video_vendor_id >> 8) & 0xff; + video_vendor_id_buffer[3] = (video_vendor_id >> 0) & 0xff; + video_vendor_id_buffer[4] = 0; + av_dict_set(&st->metadata, "vendor_id", video_vendor_id_buffer, 0); + st->codecpar->width = avio_rb16(pb); /* width */ st->codecpar->height = avio_rb16(pb); /* height */ @@ -1880,9 +1890,20 @@ static void mov_parse_stsd_audio(MOVContext *c, AVIOContext *pb, int bits_per_sample, flags; uint16_t version = avio_rb16(pb); AVDictionaryEntry *compatible_brands = av_dict_get(c->fc->metadata, "compatible_brands", NULL, AV_DICT_MATCH_CASE); + int audio_vendor_id = 0; + char audio_vendor_id_buffer[5]; avio_rb16(pb); /* revision level */ - avio_rb32(pb); /* vendor */ + audio_vendor_id = avio_rb32(pb); /* vendor */ + + /* set audio_vendor_id */ + audio_vendor_id_buffer[0] = (audio_vendor_id >> 24) & 0xff; + audio_vendor_id_buffer[1] = (audio_vendor_id >> 16) & 0xff; + audio_vendor_id_buffer[2] = (audio_vendor_id >> 8) & 0xff; + audio_vendor_id_buffer[3] = (audio_vendor_id >> 0) & 0xff; + audio_vendor_id_buffer[4] = 0; + + av_dict_set(&st->metadata, "vendor_id", audio_vendor_id_buffer, 0); st->codecpar->channels = avio_rb16(pb); /* channel count */ st->codecpar->bits_per_coded_sample = avio_rb16(pb); /* sample size */ diff --git a/tests/ref/fate/rgb24-mkv b/tests/ref/fate/rgb24-mkv index 88d22c1..ba6311d 100644 --- a/tests/ref/fate/rgb24-mkv +++ b/tests/ref/fate/rgb24-mkv @@ -1,5 +1,5 @@ -94cce0d7d5b14b4c86e74a1ca454c5aa *tests/data/fate/rgb24-mkv.matroska -58361 tests/data/fate/rgb24-mkv.matroska +29e4fffecb2002912fc05ed910679ce3 *tests/data/fate/rgb24-mkv.matroska +58390 tests/data/fate/rgb24-mkv.matroska #tb 0: 1/10 #media_type 0: video #codec_id 0: rawvideo