| Message ID | OS3PR01MB5976E53A6148794FE7773628C1D09@OS3PR01MB5976.jpnprd01.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v1,1/1] libavformat/amr.c: remove mode range check | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang <hisunzhenliang@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@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
> > On Sep 4, 2021, at 16:01, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang <hisunzhenliang@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@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. - sunzhenliang
On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang <hisunzhenliang@outlook.com> wrote: > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang <hisunzhenliang@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@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. > - sunzhenliang > > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com>，写道： > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang <hisunzhenliang@outlook.com> > wrote: > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes <h.leppkes@gmail.com> wrote: > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang <hisunzhenliang@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@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. - sunzhenliang > > - sunzhenliang > > > > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang <hisunzhenliang@outlook.com> wrote: > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com>，写道： > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang <hisunzhenliang@outlook.com > > > > wrote: > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes <h.leppkes@gmail.com> > wrote: > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > hisunzhenliang@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@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@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang <hisunzhenliang@outlook.com> > wrote: > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com>，写道： > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang <hisunzhenliang@outlook.com > > > > > > wrote: > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes <h.leppkes@gmail.com> > > wrote: > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > hisunzhenliang@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@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? > I have done fate-test, but can not submit the results with fate_recv. I have sent the public ssh key to fate-admin@ffmpeg.org, maybe it needs some time? > > > > > - sunzhenliang > > > > - sunzhenliang > > > > > > > > > > > > > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Tue, Sep 7, 2021 at 11:36 AM Sun Zhenliang <hisunzhenliang@outlook.com> wrote: > 在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang <hisunzhenliang@outlook.com > > > > wrote: > > > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com>，写道： > > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang < > hisunzhenliang@outlook.com > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes <h.leppkes@gmail.com> > > > wrote: > > > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > > hisunzhenliang@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@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? > > > I have done fate-test, but can not submit the results with fate_recv. > I have sent the public ssh key to fate-admin@ffmpeg.org, maybe it > needs some time? > I mean regressions not covered by FATE. Also you misunderstood fate-test, this patch have nothing to do with sending your fate testing reports to FFmpeg. > > > > > > > > - sunzhenliang > > > > > - sunzhenliang > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > ffmpeg-devel@ffmpeg.org > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
在 2021年9月7日 +0800 19:13，Paul B Mahol <onemda@gmail.com>，写道： > On Tue, Sep 7, 2021 at 11:36 AM Sun Zhenliang <hisunzhenliang@outlook.com> > wrote: > > > 在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > > > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang <hisunzhenliang@outlook.com > > > > > > wrote: > > > > > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com>，写道： > > > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang < > > hisunzhenliang@outlook.com > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes <h.leppkes@gmail.com> > > > > wrote: > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > > > hisunzhenliang@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@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? > > > > > I have done fate-test, but can not submit the results with fate_recv. > > I have sent the public ssh key to fate-admin@ffmpeg.org, maybe it > > needs some time? > > > > I mean regressions not covered by FATE. > > Also you misunderstood fate-test, this patch have nothing to do with > sending your fate testing reports to FFmpeg. > Sorry, could you explain more in details? I am confused. I have checked this patch, which works fine with those amr files. > > > > > > > > > > > - sunzhenliang > > > > > > - sunzhenliang > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > ffmpeg-devel mailing list > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > ffmpeg-devel@ffmpeg.org > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Tue, Sep 7, 2021 at 4:48 PM Sun Zhenliang <hisunzhenliang@outlook.com> wrote: > 在 2021年9月7日 +0800 19:13，Paul B Mahol <onemda@gmail.com>，写道： > > On Tue, Sep 7, 2021 at 11:36 AM Sun Zhenliang < > hisunzhenliang@outlook.com> > > wrote: > > > > > 在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > > > > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang < > hisunzhenliang@outlook.com > > > > > > > > wrote: > > > > > > > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang < > > > hisunzhenliang@outlook.com > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes < > h.leppkes@gmail.com> > > > > > wrote: > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > > > > hisunzhenliang@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@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? > > > > > > > I have done fate-test, but can not submit the results with fate_recv. > > > I have sent the public ssh key to fate-admin@ffmpeg.org, maybe it > > > needs some time? > > > > > > > I mean regressions not covered by FATE. > > > > Also you misunderstood fate-test, this patch have nothing to do with > > sending your fate testing reports to FFmpeg. > > > Sorry, could you explain more in details? I am confused. > > I have checked this patch, which works fine with those amr files. > What about non-amr files mentioned in tickets in git log history of this demuxer. > > > > > > > > > > > > > > - sunzhenliang > > > > > > > - sunzhenliang > > > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > ffmpeg-devel mailing list > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > > > _______________________________________________ > > > > > > ffmpeg-devel mailing list > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > ffmpeg-devel@ffmpeg.org > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
在 2021年9月8日 +0800 00:14，Paul B Mahol <onemda@gmail.com>，写道： > On Tue, Sep 7, 2021 at 4:48 PM Sun Zhenliang <hisunzhenliang@outlook.com> > wrote: > > > 在 2021年9月7日 +0800 19:13，Paul B Mahol <onemda@gmail.com>，写道： > > > On Tue, Sep 7, 2021 at 11:36 AM Sun Zhenliang < > > hisunzhenliang@outlook.com> > > > wrote: > > > > > > > 在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > > > > > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang < > > hisunzhenliang@outlook.com > > > > > > > > > > wrote: > > > > > > > > > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang < > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes < > > h.leppkes@gmail.com> > > > > > > wrote: > > > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > > > > > hisunzhenliang@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@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? > > > > > > > > > I have done fate-test, but can not submit the results with fate_recv. > > > > I have sent the public ssh key to fate-admin@ffmpeg.org, maybe it > > > > needs some time? > > > > > > > > > > I mean regressions not covered by FATE. > > > > > > Also you misunderstood fate-test, this patch have nothing to do with > > > sending your fate testing reports to FFmpeg. > > > > > Sorry, could you explain more in details? I am confused. > > > > I have checked this patch, which works fine with those amr files. > > > > What about non-amr files mentioned in tickets in git log history of this > demuxer. > Thanks for explaining, I understand now. I have checked those tickets in git history of this demuxer (tickets #7270, #7125, #6208, #3541). It works fine. > > > > > > > > > > > > > > > > > - sunzhenliang > > > > > > > > - sunzhenliang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > ffmpeg-devel mailing list > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > ffmpeg-devel mailing list > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > _______________________________________________ > > > > > > ffmpeg-devel mailing list > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > ffmpeg-devel@ffmpeg.org > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Wed, Sep 8, 2021 at 4:23 AM Sun Zhenliang <hisunzhenliang@outlook.com> wrote: > 在 2021年9月8日 +0800 00:14，Paul B Mahol <onemda@gmail.com>，写道： > > On Tue, Sep 7, 2021 at 4:48 PM Sun Zhenliang <hisunzhenliang@outlook.com > > > > wrote: > > > > > 在 2021年9月7日 +0800 19:13，Paul B Mahol <onemda@gmail.com>，写道： > > > > On Tue, Sep 7, 2021 at 11:36 AM Sun Zhenliang < > > > hisunzhenliang@outlook.com> > > > > wrote: > > > > > > > > > 在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang < > > > hisunzhenliang@outlook.com > > > > > > > > > > > > wrote: > > > > > > > > > > > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang < > > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes < > > > h.leppkes@gmail.com> > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > > > > > > hisunzhenliang@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@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? > > > > > > > > > > > I have done fate-test, but can not submit the results with > fate_recv. > > > > > I have sent the public ssh key to fate-admin@ffmpeg.org, maybe it > > > > > needs some time? > > > > > > > > > > > > > I mean regressions not covered by FATE. > > > > > > > > Also you misunderstood fate-test, this patch have nothing to do with > > > > sending your fate testing reports to FFmpeg. > > > > > > > Sorry, could you explain more in details? I am confused. > > > > > > I have checked this patch, which works fine with those amr files. > > > > > > > What about non-amr files mentioned in tickets in git log history of this > > demuxer. > > > Thanks for explaining, I understand now. > I have checked those tickets in git history of this demuxer > (tickets #7270, #7125, #6208, #3541). > It works fine. > So you actually compiled ffmpeg with midi support at tested that those midi files are working fine? > > > > > > > > > > > > > > > > > > > > - sunzhenliang > > > > > > > > > - sunzhenliang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject > "unsubscribe". > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > ffmpeg-devel mailing list > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > _______________________________________________ > > > > > > > ffmpeg-devel mailing list > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > > > _______________________________________________ > > > > > > ffmpeg-devel mailing list > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > ffmpeg-devel@ffmpeg.org > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
在 2021年9月8日 +0800 16:37，Paul B Mahol <onemda@gmail.com>，写道： > On Wed, Sep 8, 2021 at 4:23 AM Sun Zhenliang <hisunzhenliang@outlook.com> > wrote: > > > 在 2021年9月8日 +0800 00:14，Paul B Mahol <onemda@gmail.com>，写道： > > > On Tue, Sep 7, 2021 at 4:48 PM Sun Zhenliang <hisunzhenliang@outlook.com > > > > > > wrote: > > > > > > > 在 2021年9月7日 +0800 19:13，Paul B Mahol <onemda@gmail.com>，写道： > > > > > On Tue, Sep 7, 2021 at 11:36 AM Sun Zhenliang < > > > > hisunzhenliang@outlook.com> > > > > > wrote: > > > > > > > > > > > 在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang < > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang < > > > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes < > > > > h.leppkes@gmail.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > > > > > > > hisunzhenliang@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@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? > > > > > > > > > > > > > I have done fate-test, but can not submit the results with > > fate_recv. > > > > > > I have sent the public ssh key to fate-admin@ffmpeg.org, maybe it > > > > > > needs some time? > > > > > > > > > > > > > > > > I mean regressions not covered by FATE. > > > > > > > > > > Also you misunderstood fate-test, this patch have nothing to do with > > > > > sending your fate testing reports to FFmpeg. > > > > > > > > > Sorry, could you explain more in details? I am confused. > > > > > > > > I have checked this patch, which works fine with those amr files. > > > > > > > > > > What about non-amr files mentioned in tickets in git log history of this > > > demuxer. > > > > > Thanks for explaining, I understand now. > > I have checked those tickets in git history of this demuxer > > (tickets #7270, #7125, #6208, #3541). > > It works fine. > > > > So you actually compiled ffmpeg with midi support at tested that those midi > files are working fine? > > No, I just tested those midi file added in #7270, which were not misdetected as amr. > > > > > > > > > > > > > > > > > > > > > > > - sunzhenliang > > > > > > > > > > - sunzhenliang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject > > "unsubscribe". > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > _______________________________________________ > > > > > > > > ffmpeg-devel mailing list > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > ffmpeg-devel mailing list > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > _______________________________________________ > > > > > > ffmpeg-devel mailing list > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > ffmpeg-devel@ffmpeg.org > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Wed, Sep 8, 2021 at 10:56 AM Sun Zhenliang <hisunzhenliang@outlook.com> wrote: > 在 2021年9月8日 +0800 16:37，Paul B Mahol <onemda@gmail.com>，写道： > > On Wed, Sep 8, 2021 at 4:23 AM Sun Zhenliang <hisunzhenliang@outlook.com > > > > wrote: > > > > > 在 2021年9月8日 +0800 00:14，Paul B Mahol <onemda@gmail.com>，写道： > > > > On Tue, Sep 7, 2021 at 4:48 PM Sun Zhenliang < > hisunzhenliang@outlook.com > > > > > > > > wrote: > > > > > > > > > 在 2021年9月7日 +0800 19:13，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > On Tue, Sep 7, 2021 at 11:36 AM Sun Zhenliang < > > > > > hisunzhenliang@outlook.com> > > > > > > wrote: > > > > > > > > > > > > > 在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > > > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang < > > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com > >，写道： > > > > > > > > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang < > > > > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes < > > > > > h.leppkes@gmail.com> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > > > > > > > > hisunzhenliang@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@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? > > > > > > > > > > > > > > > I have done fate-test, but can not submit the results with > > > fate_recv. > > > > > > > I have sent the public ssh key to fate-admin@ffmpeg.org, > maybe it > > > > > > > needs some time? > > > > > > > > > > > > > > > > > > > I mean regressions not covered by FATE. > > > > > > > > > > > > Also you misunderstood fate-test, this patch have nothing to do > with > > > > > > sending your fate testing reports to FFmpeg. > > > > > > > > > > > Sorry, could you explain more in details? I am confused. > > > > > > > > > > I have checked this patch, which works fine with those amr files. > > > > > > > > > > > > > What about non-amr files mentioned in tickets in git log history of > this > > > > demuxer. > > > > > > > Thanks for explaining, I understand now. > > > I have checked those tickets in git history of this demuxer > > > (tickets #7270, #7125, #6208, #3541). > > > It works fine. > > > > > > > So you actually compiled ffmpeg with midi support at tested that those > midi > > files are working fine? > > > > > No, I just tested those midi file added in #7270, which were not > misdetected as amr. > Sorry, That means you have not tested that at all. > > > > > > > > > > > > > > > > > > > > > > > > > > - sunzhenliang > > > > > > > > > > > - sunzhenliang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject > > > "unsubscribe". > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject > "unsubscribe". > > > > > > > > > _______________________________________________ > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject > "unsubscribe". > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > ffmpeg-devel mailing list > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > _______________________________________________ > > > > > > > ffmpeg-devel mailing list > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > > > _______________________________________________ > > > > > > ffmpeg-devel mailing list > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > ffmpeg-devel@ffmpeg.org > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". >
On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang <hisunzhenliang@outlook.com> wrote: > > 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. > Can you answer the second question? Does this actually fix any files? If so, can you provide one? As mentioned above, correct or not, this patch makes the probing accept very loose bit patterns and is likely rather prone to misdetections, so no matter if its more "correct", there are other considerations here. So instead, lets start with seeing what it actually fixes. - Hendrik
在 2021年9月8日 +0800 17:55，Hendrik Leppkes <h.leppkes@gmail.com>，写道： > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang <hisunzhenliang@outlook.com> wrote: > > > > 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. > > > > Can you answer the second question? Does this actually fix any files? > If so, can you provide one? > > As mentioned above, correct or not, this patch makes the probing > accept very loose bit patterns and is likely rather prone to > misdetections, so no matter if its more "correct", there are other > considerations here. So instead, lets start with seeing what it > actually fixes. Ok, there is the file that this patch fixed. It’s made by some kind of phone with header. I removed it’s header to test the probing at raw amr files. https://github.com/HiSunzhenliang/patch/blob/main/ffmpeg/silence-head.amrnb And this is ffmpeg log. https://github.com/HiSunzhenliang/patch/blob/main/ffmpeg/ffmpeg_i.log > > - Hendrik > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
在 2021年9月8日 +0800 17:10，Paul B Mahol <onemda@gmail.com>，写道： > On Wed, Sep 8, 2021 at 10:56 AM Sun Zhenliang <hisunzhenliang@outlook.com> > wrote: > > > 在 2021年9月8日 +0800 16:37，Paul B Mahol <onemda@gmail.com>，写道： > > > On Wed, Sep 8, 2021 at 4:23 AM Sun Zhenliang <hisunzhenliang@outlook.com > > > > > > wrote: > > > > > > > 在 2021年9月8日 +0800 00:14，Paul B Mahol <onemda@gmail.com>，写道： > > > > > On Tue, Sep 7, 2021 at 4:48 PM Sun Zhenliang < > > hisunzhenliang@outlook.com > > > > > > > > > > wrote: > > > > > > > > > > > 在 2021年9月7日 +0800 19:13，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > > On Tue, Sep 7, 2021 at 11:36 AM Sun Zhenliang < > > > > > > hisunzhenliang@outlook.com> > > > > > > > wrote: > > > > > > > > > > > > > > > 在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > > > > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang < > > > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com > > > ，写道： > > > > > > > > > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang < > > > > > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes < > > > > > > h.leppkes@gmail.com> > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > > > > > > > > > hisunzhenliang@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@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? > > > > > > > > > > > > > > > > > I have done fate-test, but can not submit the results with > > > > fate_recv. > > > > > > > > I have sent the public ssh key to fate-admin@ffmpeg.org, > > maybe it > > > > > > > > needs some time? > > > > > > > > > > > > > > > > > > > > > > I mean regressions not covered by FATE. > > > > > > > > > > > > > > Also you misunderstood fate-test, this patch have nothing to do > > with > > > > > > > sending your fate testing reports to FFmpeg. > > > > > > > > > > > > > Sorry, could you explain more in details? I am confused. > > > > > > > > > > > > I have checked this patch, which works fine with those amr files. > > > > > > > > > > > > > > > > What about non-amr files mentioned in tickets in git log history of > > this > > > > > demuxer. > > > > > > > > > Thanks for explaining, I understand now. > > > > I have checked those tickets in git history of this demuxer > > > > (tickets #7270, #7125, #6208, #3541). > > > > It works fine. > > > > > > > > > > So you actually compiled ffmpeg with midi support at tested that those > > midi > > > files are working fine? > > > > > > > > No, I just tested those midi file added in #7270, which were not > > misdetected as amr. > > > > Sorry, That means you have not tested that at all. > > Sorry, I am not familiar with midi. Is it ok to just compile ffmpeg with libmodplug? because I just find .mid is defined in libmodplug.c. Here is the ffmpeg log to test midi file in #7270. ~/Downloads ❯ ffmpeg -i not_amr.mid -loglevel 48 ffmpeg version N-103542-gb9426f371a Copyright (c) 2000-2021 the FFmpeg developers built with gcc 9 (Ubuntu 9.3.0-17ubuntu1~20.04) configuration: --prefix=/home/sun/.local --enable-shared --enable-sdl --enable-gpl --enable-version3 --enable-libmp3lame --enable-debug --enable-libopencore-amrnb --enable-libopencore-amrwb --enable-libvo-amrwbenc --enable-libx265 --enable-libx264 --disable-x86asm --enable-libmodplug libavutil 57. 4.101 / 57. 4.101 libavcodec 59. 7.101 / 59. 7.101 libavformat 59. 5.100 / 59. 5.100 libavdevice 59. 0.101 / 59. 0.101 libavfilter 8. 7.101 / 8. 7.101 libswscale 6. 1.100 / 6. 1.100 libswresample 4. 0.100 / 4. 0.100 libpostproc 56. 0.100 / 56. 0.100 Splitting the commandline. Reading option '-i' ... matched as input url with argument 'not_amr.mid'. Reading option '-loglevel' ... matched as option 'loglevel' (set logging level) with argument '48'. Finished splitting the commandline. Parsing a group of options: global . Applying option loglevel (set logging level) with argument 48. Successfully parsed a group of options. Parsing a group of options: input url not_amr.mid. Successfully parsed a group of options. Opening an input file: not_amr.mid. [NULL @ 0x55bafaa57e40] Opening 'not_amr.mid' for reading [file @ 0x55bafaa58780] Setting default whitelist 'file,crypto,data' [AVIOContext @ 0x55bafaa60b40] Statistics: 4435 bytes read, 0 seeks not_amr.mid: Invalid data found when processing input Is it ok? > > > > > > > > > > > > > > > > > > > > > > > > > > > > > - sunzhenliang > > > > > > > > > > > > - sunzhenliang > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject > > > > "unsubscribe". > > > > > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject > > "unsubscribe". > > > > > > > > > > _______________________________________________ > > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject > > "unsubscribe". > > > > > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > > > ffmpeg-devel mailing list > > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > _______________________________________________ > > > > > > > > ffmpeg-devel mailing list > > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > > > > > _______________________________________________ > > > > > > > ffmpeg-devel mailing list > > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > _______________________________________________ > > > > > > ffmpeg-devel mailing list > > > > > > ffmpeg-devel@ffmpeg.org > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > > > > > _______________________________________________ > > > > > ffmpeg-devel mailing list > > > > > ffmpeg-devel@ffmpeg.org > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > > > To unsubscribe, visit link above, or email > > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > _______________________________________________ > > > > ffmpeg-devel mailing list > > > > ffmpeg-devel@ffmpeg.org > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > > > To unsubscribe, visit link above, or email > > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > > > > > _______________________________________________ > > > ffmpeg-devel mailing list > > > ffmpeg-devel@ffmpeg.org > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > > > To unsubscribe, visit link above, or email > > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
在 2021年9月8日 +0800 22:12，Sun Zhenliang <hisunzhenliang@outlook.com>，写道： > 在 2021年9月8日 +0800 17:10，Paul B Mahol <onemda@gmail.com>，写道： > > On Wed, Sep 8, 2021 at 10:56 AM Sun Zhenliang <hisunzhenliang@outlook.com> > > wrote: > > > > > 在 2021年9月8日 +0800 16:37，Paul B Mahol <onemda@gmail.com>，写道： > > > > On Wed, Sep 8, 2021 at 4:23 AM Sun Zhenliang <hisunzhenliang@outlook.com > > > > > > > > wrote: > > > > > > > > > 在 2021年9月8日 +0800 00:14，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > On Tue, Sep 7, 2021 at 4:48 PM Sun Zhenliang < > > > hisunzhenliang@outlook.com > > > > > > > > > > > > wrote: > > > > > > > > > > > > > 在 2021年9月7日 +0800 19:13，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > > > On Tue, Sep 7, 2021 at 11:36 AM Sun Zhenliang < > > > > > > > hisunzhenliang@outlook.com> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > 在 2021年9月7日 +0800 14:16，Paul B Mahol <onemda@gmail.com>，写道： > > > > > > > > > > On Tue, Sep 7, 2021 at 7:34 AM Sun Zhenliang < > > > > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > 在 2021年9月7日 +0800 00:55，Paul B Mahol <onemda@gmail.com > > > > ，写道： > > > > > > > > > > > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang < > > > > > > > > > hisunzhenliang@outlook.com > > > > > > > > > > > > > > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sep 4, 2021, at 16:01, Hendrik Leppkes < > > > > > > > h.leppkes@gmail.com> > > > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > On Sat, Sep 4, 2021 at 9:10 AM sunzhenliang < > > > > > > > > > > > hisunzhenliang@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@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? > > > > > > > > > > > > > > > > > > > I have done fate-test, but can not submit the results with > > > > > fate_recv. > > > > > > > > > I have sent the public ssh key to fate-admin@ffmpeg.org, > > > maybe it > > > > > > > > > needs some time? > > > > > > > > > > > > > > > > > > > > > > > > > I mean regressions not covered by FATE. > > > > > > > > > > > > > > > > Also you misunderstood fate-test, this patch have nothing to do > > > with > > > > > > > > sending your fate testing reports to FFmpeg. > > > > > > > > > > > > > > > Sorry, could you explain more in details? I am confused. > > > > > > > > > > > > > > I have checked this patch, which works fine with those amr files. > > > > > > > > > > > > > > > > > > > What about non-amr files mentioned in tickets in git log history of > > > this > > > > > > demuxer. > > > > > > > > > > > Thanks for explaining, I understand now. > > > > > I have checked those tickets in git history of this demuxer > > > > > (tickets #7270, #7125, #6208, #3541). > > > > > It works fine. > > > > > > > > > > > > > So you actually compiled ffmpeg with midi support at tested that those > > > midi > > > > files are working fine? > > > > > > > > > > > No, I just tested those midi file added in #7270, which were not > > > misdetected as amr. > > > > > > > Sorry, That means you have not tested that at all. > > > > > Sorry, I am not familiar with midi. Is it ok to just compile ffmpeg with libmodplug? > because I just find .mid is defined in libmodplug.c. > > Here is the ffmpeg log to test midi file in #7270. > > ~/Downloads ❯ ffmpeg -i not_amr.mid -loglevel 48 > ffmpeg version N-103542-gb9426f371a Copyright (c) 2000-2021 the FFmpeg developers > built with gcc 9 (Ubuntu 9.3.0-17ubuntu1~20.04) > configuration: --prefix=/home/sun/.local --enable-shared --enable-sdl --enable-gpl > --enable-version3 --enable-libmp3lame --enable-debug --enable-libopencore-amrnb > --enable-libopencore-amrwb --enable-libvo-amrwbenc --enable-libx265 --enable-libx264 > --disable-x86asm --enable-libmodplug > libavutil 57. 4.101 / 57. 4.101 > libavcodec 59. 7.101 / 59. 7.101 > libavformat 59. 5.100 / 59. 5.100 > libavdevice 59. 0.101 / 59. 0.101 > libavfilter 8. 7.101 / 8. 7.101 > libswscale 6. 1.100 / 6. 1.100 > libswresample 4. 0.100 / 4. 0.100 > libpostproc 56. 0.100 / 56. 0.100 > Splitting the commandline. > Reading option '-i' ... matched as input url with argument 'not_amr.mid'. > Reading option '-loglevel' ... matched as option 'loglevel' (set logging level) with argument '48'. > Finished splitting the commandline. > Parsing a group of options: global . > Applying option loglevel (set logging level) with argument 48. > Successfully parsed a group of options. > Parsing a group of options: input url not_amr.mid. > Successfully parsed a group of options. > Opening an input file: not_amr.mid. > [NULL @ 0x55bafaa57e40] Opening 'not_amr.mid' for reading > [file @ 0x55bafaa58780] Setting default whitelist 'file,crypto,data' > [AVIOContext @ 0x55bafaa60b40] Statistics: 4435 bytes read, 0 seeks > not_amr.mid: Invalid data found when processing input > > Is it ok? > > > > > > > > > > > > > > > > > > > > > Ping
在 2021年9月8日 +0800 21:48，Sun Zhenliang <hisunzhenliang@outlook.com>，写道： > 在 2021年9月8日 +0800 17:55，Hendrik Leppkes <h.leppkes@gmail.com>，写道： > > On Sat, Sep 4, 2021 at 1:52 PM Sun Zhenliang <hisunzhenliang@outlook.com> wrote: > > > > > > 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. > > > > > > > Can you answer the second question? Does this actually fix any files? > > If so, can you provide one? > > > > As mentioned above, correct or not, this patch makes the probing > > accept very loose bit patterns and is likely rather prone to > > misdetections, so no matter if its more "correct", there are other > > considerations here. So instead, lets start with seeing what it > > actually fixes. > > Ok, there is the file that this patch fixed. It’s made by some kind of phone with header. > I removed it’s header to test the probing at raw amr files. > https://github.com/HiSunzhenliang/patch/blob/main/ffmpeg/silence-head.amrnb > And this is ffmpeg log. > https://github.com/HiSunzhenliang/patch/blob/main/ffmpeg/ffmpeg_i.log > > Ping > > - Hendrik > > _______________________________________________ > > ffmpeg-devel mailing list > > ffmpeg-devel@ffmpeg.org > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > > > To unsubscribe, visit link above, or email > > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
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--) {
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@outlook.com> --- libavformat/amr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)