| Message ID | 20170518004941.5140-2-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, May 17, 2017 at 09:49:39PM -0300, James Almer wrote: > As defined in "VP Codec ISO Media File Format Binding v1.0" > https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > libavformat/movenc.c | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index cd436df7a4..eab7bbc8a7 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -1154,6 +1154,27 @@ static int mov_write_smdm_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra > return update_size(pb, pos); > } > > +static int mov_write_coll_tag(AVIOContext *pb, MOVTrack *track) > +{ > + int size = 0; > + int64_t pos; > + const AVContentLightMetadata *coll = > + (const AVContentLightMetadata *) av_stream_get_side_data(track->st, > + AV_PKT_DATA_CONTENT_LIGHT_LEVEL, > + &size); > + if (!size) > + return 0; Is there anything that checks the validity of size for a AVContentLightMetadata ? (that is, is this checked anywhere from side data creation to its use here) If not then this can be too small and crash [...]
On 5/26/2017 7:56 PM, Michael Niedermayer wrote: > On Wed, May 17, 2017 at 09:49:39PM -0300, James Almer wrote: >> As defined in "VP Codec ISO Media File Format Binding v1.0" >> https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md >> >> Signed-off-by: James Almer <jamrial@gmail.com> >> --- >> libavformat/movenc.c | 22 ++++++++++++++++++++++ >> 1 file changed, 22 insertions(+) >> >> diff --git a/libavformat/movenc.c b/libavformat/movenc.c >> index cd436df7a4..eab7bbc8a7 100644 >> --- a/libavformat/movenc.c >> +++ b/libavformat/movenc.c >> @@ -1154,6 +1154,27 @@ static int mov_write_smdm_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra >> return update_size(pb, pos); >> } >> >> +static int mov_write_coll_tag(AVIOContext *pb, MOVTrack *track) >> +{ >> + int size = 0; >> + int64_t pos; >> + const AVContentLightMetadata *coll = >> + (const AVContentLightMetadata *) av_stream_get_side_data(track->st, >> + AV_PKT_DATA_CONTENT_LIGHT_LEVEL, >> + &size); >> + if (!size) >> + return 0; > > Is there anything that checks the validity of size for a > AVContentLightMetadata ? > (that is, is this checked anywhere from side data creation to > its use here) If created by av_content_light_metadata_alloc() and the returned size passed to av_stream_add_side_data(), then it's guaranteed to be correct. Any other case, guess it depends on if whatever created the side data ignored the "size not part of the ABI" warning and sizeof'd the struct or not. > > If not then this can be too small and crash That's apparently a risk on every current usage of side data (mastering, stereo3d, spherical, content light, etc) in demuxing code, except those doing sizeof() against what the doxy says. I suggested once adding functions that return the size of the struct, much like the alloc() ones do. Some thought it was a good solution, others thought it was added complexity to a simple API, and others wanted to rewrite the entire side data handling code API from scratch. In the end, none of them were done.
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index cd436df7a4..eab7bbc8a7 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -1154,6 +1154,27 @@ static int mov_write_smdm_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra return update_size(pb, pos); } +static int mov_write_coll_tag(AVIOContext *pb, MOVTrack *track) +{ + int size = 0; + int64_t pos; + const AVContentLightMetadata *coll = + (const AVContentLightMetadata *) av_stream_get_side_data(track->st, + AV_PKT_DATA_CONTENT_LIGHT_LEVEL, + &size); + if (!size) + return 0; + + pos = avio_tell(pb); + + avio_wb32(pb, 0); + ffio_wfourcc(pb, "CoLL"); + avio_wb32(pb, 0); /* version & flags */ + avio_wb16(pb, coll->MaxCLL); + avio_wb16(pb, coll->MaxFALL); + return update_size(pb, pos); +} + static int mov_write_hvcc_tag(AVIOContext *pb, MOVTrack *track) { int64_t pos = avio_tell(pb); @@ -2001,6 +2022,7 @@ static int mov_write_video_tag(AVIOContext *pb, MOVMuxContext *mov, MOVTrack *tr } else if (track->par->codec_id == AV_CODEC_ID_VP9) { mov_write_vpcc_tag(mov->fc, pb, track); mov_write_smdm_tag(mov->fc, pb, track); + mov_write_coll_tag(pb, track); } else if (track->par->codec_id == AV_CODEC_ID_VC1 && track->vos_len > 0) mov_write_dvc1_tag(pb, track); else if (track->par->codec_id == AV_CODEC_ID_VP6F ||
As defined in "VP Codec ISO Media File Format Binding v1.0" https://github.com/webmproject/vp9-dash/blob/master/VPCodecISOMediaFileFormatBinding.md Signed-off-by: James Almer <jamrial@gmail.com> --- libavformat/movenc.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)