diff mbox series

[FFmpeg-devel] avformat/movenc: fix writing dOps atoms

Message ID 20210415043942.5053-1-jamrial@gmail.com
State Accepted
Commit bc010612318ea0bbd31d4b92cbb90ccfbed09631
Headers show
Series [FFmpeg-devel] avformat/movenc: fix writing dOps atoms
Related show

Checks

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

Commit Message

James Almer April 15, 2021, 4:39 a.m. UTC
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(-)

Comments

James Almer April 16, 2021, 1:46 p.m. UTC | #1
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 mbox series

Patch

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);
 }