| Message ID | 20210415043942.5053-1-jamrial@gmail.com |
|---|---|
| State | Accepted |
| Commit | bc010612318ea0bbd31d4b92cbb90ccfbed09631 |
| Headers | show |
| Series | [FFmpeg-devel] avformat/movenc: fix writing dOps atoms | expand |
| Context | Check | Description |
|---|---|---|
| andriy/x86_make | success | Make finished |
| andriy/x86_make_fate | success | Make fate finished |
| andriy/PPC64_make | success | Make finished |
| andriy/PPC64_make_fate | success | Make fate finished |
On 4/15/2021 1:39 AM, James Almer wrote: > Don't blindly copy all bytes in extradata past ChannelMappingFamily. Instead > check if ChannelMappingFamily is not 0 and then only write the correct amount > of bytes from ChannelMappingTable, as defined in the spec[1]. > > Should fix ticket #9190. > > [1] https://opus-codec.org/docs/opus_in_isobmff.html#4.3.2 > > Signed-off-by: James Almer <jamrial@gmail.com> > --- > Compared to "avformat/mpegts: set correct extradata size for Opus streams", > which only ensured the mpets demuxer would output something the mov muxer would > not mishandle, this is a proper fix for the ticket, making the muxer spec > compliant. > > libavformat/movenc.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/libavformat/movenc.c b/libavformat/movenc.c > index 0b620a802d..8e6ed817d8 100644 > --- a/libavformat/movenc.c > +++ b/libavformat/movenc.c > @@ -797,6 +797,7 @@ static int mov_write_dfla_tag(AVIOContext *pb, MOVTrack *track) > static int mov_write_dops_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track) > { > int64_t pos = avio_tell(pb); > + int channels, channel_map; > avio_wb32(pb, 0); > ffio_wfourcc(pb, "dOps"); > avio_w8(pb, 0); /* Version */ > @@ -807,12 +808,22 @@ static int mov_write_dops_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra > /* extradata contains an Ogg OpusHead, other than byte-ordering and > OpusHead's preceeding magic/version, OpusSpecificBox is currently > identical. */ > - avio_w8(pb, AV_RB8(track->par->extradata + 9)); /* OuputChannelCount */ > + channels = AV_RB8(track->par->extradata + 9); > + channel_map = AV_RB8(track->par->extradata + 18); > + > + avio_w8(pb, channels); /* OuputChannelCount */ > avio_wb16(pb, AV_RL16(track->par->extradata + 10)); /* PreSkip */ > avio_wb32(pb, AV_RL32(track->par->extradata + 12)); /* InputSampleRate */ > avio_wb16(pb, AV_RL16(track->par->extradata + 16)); /* OutputGain */ > + avio_w8(pb, channel_map); /* ChannelMappingFamily */ > /* Write the rest of the header out without byte-swapping. */ > - avio_write(pb, track->par->extradata + 18, track->par->extradata_size - 18); > + if (channel_map) { > + if (track->par->extradata_size < 21 + channels) { > + av_log(s, AV_LOG_ERROR, "invalid extradata size\n"); > + return AVERROR_INVALIDDATA; > + } > + avio_write(pb, track->par->extradata + 19, 2 + channels); /* ChannelMappingTable */ > + } > > return update_size(pb, pos); > } Will apply.
diff --git a/libavformat/movenc.c b/libavformat/movenc.c index 0b620a802d..8e6ed817d8 100644 --- a/libavformat/movenc.c +++ b/libavformat/movenc.c @@ -797,6 +797,7 @@ static int mov_write_dfla_tag(AVIOContext *pb, MOVTrack *track) static int mov_write_dops_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *track) { int64_t pos = avio_tell(pb); + int channels, channel_map; avio_wb32(pb, 0); ffio_wfourcc(pb, "dOps"); avio_w8(pb, 0); /* Version */ @@ -807,12 +808,22 @@ static int mov_write_dops_tag(AVFormatContext *s, AVIOContext *pb, MOVTrack *tra /* extradata contains an Ogg OpusHead, other than byte-ordering and OpusHead's preceeding magic/version, OpusSpecificBox is currently identical. */ - avio_w8(pb, AV_RB8(track->par->extradata + 9)); /* OuputChannelCount */ + channels = AV_RB8(track->par->extradata + 9); + channel_map = AV_RB8(track->par->extradata + 18); + + avio_w8(pb, channels); /* OuputChannelCount */ avio_wb16(pb, AV_RL16(track->par->extradata + 10)); /* PreSkip */ avio_wb32(pb, AV_RL32(track->par->extradata + 12)); /* InputSampleRate */ avio_wb16(pb, AV_RL16(track->par->extradata + 16)); /* OutputGain */ + avio_w8(pb, channel_map); /* ChannelMappingFamily */ /* Write the rest of the header out without byte-swapping. */ - avio_write(pb, track->par->extradata + 18, track->par->extradata_size - 18); + if (channel_map) { + if (track->par->extradata_size < 21 + channels) { + av_log(s, AV_LOG_ERROR, "invalid extradata size\n"); + return AVERROR_INVALIDDATA; + } + avio_write(pb, track->par->extradata + 19, 2 + channels); /* ChannelMappingTable */ + } return update_size(pb, pos); }
Don't blindly copy all bytes in extradata past ChannelMappingFamily. Instead check if ChannelMappingFamily is not 0 and then only write the correct amount of bytes from ChannelMappingTable, as defined in the spec[1]. Should fix ticket #9190. [1] https://opus-codec.org/docs/opus_in_isobmff.html#4.3.2 Signed-off-by: James Almer <jamrial@gmail.com> --- Compared to "avformat/mpegts: set correct extradata size for Opus streams", which only ensured the mpets demuxer would output something the mov muxer would not mishandle, this is a proper fix for the ticket, making the muxer spec compliant. libavformat/movenc.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)