[FFmpeg-devel] [PATCH v1 1/1] libavformat/amr.c: remove mode range check

[Paul B Mahol]

On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang <hisunzhenliang at outlook.com>
wrote:

> 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda at gmail.com>，写道：
> > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang <hisunzhenliang at outlook.com
> >
> > wrote:
> >
> > >
> > > >
> > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes <h.leppkes at gmail.com>
> wrote:
> > > >
> > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang <
> hisunzhenliang at outlook.com>
> > > wrote:
> > > > >
> > > > > Those comfort noise frames and empty frames should be
> > > > > considered the correct frame. And amr.c/amr_read_packet()
> > > > > also takes them as correct frames too.
> > > > >
> > > > > Signed-off-by: sunzhenliang <hisunzhenliang at outlook.com>
> > > > > ---
> > > > > libavformat/amr.c | 4 ++--
> > > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/libavformat/amr.c b/libavformat/amr.c
> > > > > index 836b276fd5..540d8033d8 100644
> > > > > --- a/libavformat/amr.c
> > > > > +++ b/libavformat/amr.c
> > > > > @@ -178,7 +178,7 @@ static int amrnb_probe(const AVProbeData *p)
> > > > >
> > > > > while (i < p->buf_size) {
> > > > > mode = b[i] >> 3 & 0x0F;
> > > > > - if (mode < 9 && (b[i] & 0x4) == 0x4) {
> > > > > + if ((b[i] & 0x4) == 0x4) {
> > > > > int last = b[i];
> > > > > int size = amrnb_packed_size[mode];
> > > > > while (size--) {
> > > > > @@ -234,7 +234,7 @@ static int amrwb_probe(const AVProbeData *p)
> > > > >
> > > > > while (i < p->buf_size) {
> > > > > mode = b[i] >> 3 & 0x0F;
> > > > > - if (mode < 10 && (b[i] & 0x4) == 0x4) {
> > > > > + if ((b[i] & 0x4) == 0x4) {
> > > > > int last = b[i];
> > > > > int size = amrwb_packed_size[mode];
> > > > > while (size--) {
> > > >
> > > > This is just probing, and expecting to find a significant amount of
> > > > non-empty/non-noise frames seems like something you would want in
> > > > probing.
> > > > Does this actually fix detection of any samples?
> > > >
> > > > It seems like this has the potential of quite substantially reducing
> > > > the number of checked bits and thus invalid detections.
> > > >
> > > > - Hendrik
> > > > ____________________________________________
> > >
> > > Thanks for reviewing.
> > >
> > > Probing is expecting to find “correct” frames, which includes those
> kinds
> > > of comfort noise and NO_DATA frames mentioned in 3GPP Specifications.
> > >
> > > Besides, those frames are considered as normal frames while reading
> > > packets. It seems better to take the same standard of frames in both
> > > reading and probing.
> > >
> > >
> > Yes, but in past IIRC it caused misdetection when probing.
> But it may cause misdetection when probing audios with many
> comfort noise and NO_DATA frames. Maybe we should stick to
> 3GPP Specifications.
>

And at same time not broke probing for more more important formats.
Have you checked that your patch does not cause regressions?


>
> - sunzhenliang
> > > - sunzhenliang
> > >
> > >
> > >
> > > _______________________________________________
> > > ffmpeg-devel mailing list
> > > ffmpeg-devel at ffmpeg.org
> > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > >
> > > To unsubscribe, visit link above, or email
> > > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> > >
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> > To unsubscribe, visit link above, or email
> > ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>