diff mbox series

[FFmpeg-devel,v2,2/8] avformat/mov: check that pcmC box is of the expected type

Message ID tencent_ED661E90A326F4E7CBEA89532B5C7F4BBE06@qq.com
State Accepted
Commit adca877acb930faf1a5d686af93b9f657cebf1b5
Headers show
Series [FFmpeg-devel,v2,1/8] avformat/movenc: add PCM in mp4 support | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Zhao Zhili Feb. 24, 2023, 6:28 p.m. UTC 
From: Jan Ekström <jeebjp@gmail.com>

As per 23003-5:2020 this box is defined as
PCMConfig extends FullBox(‘pcmC’, version = 0, 0), which means
that version is 0 and flags should be zero.
---
 libavformat/mov.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

Comments

Tomas Härdin Feb. 27, 2023, 1:36 p.m. UTC | #1 
lör 2023-02-25 klockan 02:28 +0800 skrev Zhao Zhili:
> From: Jan Ekström <jeebjp@gmail.com>
> 
> As per 23003-5:2020 this box is defined as
> PCMConfig extends FullBox(‘pcmC’, version = 0, 0), which means
> that version is 0 and flags should be zero.
> ---
>  libavformat/mov.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

Probably OK, but I'm not versed i the version problems relating to this
tag. Should we also support version > 0?

/Tomas
Jan Ekström Feb. 27, 2023, 4:26 p.m. UTC | #2 
On Mon, Feb 27, 2023 at 3:36 PM Tomas Härdin <git@haerdin.se> wrote:
>
> lör 2023-02-25 klockan 02:28 +0800 skrev Zhao Zhili:
> > From: Jan Ekström <jeebjp@gmail.com>
> >
> > As per 23003-5:2020 this box is defined as
> > PCMConfig extends FullBox(‘pcmC’, version = 0, 0), which means
> > that version is 0 and flags should be zero.
> > ---
> >  libavformat/mov.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
>
> Probably OK, but I'm not versed i the version problems relating to this
> tag. Should we also support version > 0?
>
> /Tomas

pcmC only has version 0 and flags coded to zero, thankfully

The channel layout box is where the "fun" begins with version 1 as
well as this following quote:

> When authoring, version 1 should be preferred over version 0.
> Version 1 conveys the channel ordering, which is not always the case for
> version 0. Version 1 should be used to convey the base channel count for DRC.

Jan
Jan Ekström March 5, 2023, 10 p.m. UTC | #3 
On Mon, Feb 27, 2023 at 6:26 PM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Mon, Feb 27, 2023 at 3:36 PM Tomas Härdin <git@haerdin.se> wrote:
> >
> > lör 2023-02-25 klockan 02:28 +0800 skrev Zhao Zhili:
> > > From: Jan Ekström <jeebjp@gmail.com>
> > >
> > > As per 23003-5:2020 this box is defined as
> > > PCMConfig extends FullBox(‘pcmC’, version = 0, 0), which means
> > > that version is 0 and flags should be zero.
> > > ---
> > >  libavformat/mov.c | 13 +++++++++++--
> > >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > Probably OK, but I'm not versed i the version problems relating to this
> > tag. Should we also support version > 0?
> >
> > /Tomas
>
> pcmC only has version 0 and flags coded to zero, thankfully
>
> The channel layout box is where the "fun" begins with version 1 as
> well as this following quote:
>
> > When authoring, version 1 should be preferred over version 0.
> > Version 1 conveys the channel ordering, which is not always the case for
> > version 0. Version 1 should be used to convey the base channel count for DRC.

Applied as adca877acb930faf1a5d686af93b9f657cebf1b5 to make this set
under review shorter.

Jan
diff mbox series

Patch

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 8af564ed61..cdd44a9e44 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -1590,14 +1590,23 @@  static int mov_read_enda(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 static int mov_read_pcmc(MOVContext *c, AVIOContext *pb, MOVAtom atom)
 {
     int format_flags;
+    int version, flags;
 
     if (atom.size < 6) {
         av_log(c->fc, AV_LOG_ERROR, "Empty pcmC box\n");
         return AVERROR_INVALIDDATA;
     }
 
-    avio_r8(pb);    // version
-    avio_rb24(pb);  // flags
+    version = avio_r8(pb);
+    flags   = avio_rb24(pb);
+
+    if (version != 0 || flags != 0) {
+        av_log(c->fc, AV_LOG_ERROR,
+               "Unsupported 'pcmC' box with version %d, flags: %x",
+               version, flags);
+        return AVERROR_INVALIDDATA;
+    }
+
     format_flags = avio_r8(pb);
     if (format_flags == 1) // indicates little-endian format. If not present, big-endian format is used
         set_last_stream_little_endian(c->fc);