| Message ID | 20191106090045.21696-1-lance.lmwang@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Am Mi., 6. Nov. 2019 um 10:01 Uhr schrieb <lance.lmwang@gmail.com>: > > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavformat/utils.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 8196442dd1..8381498e2b 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -367,8 +367,7 @@ static int set_codec_from_probe_data(AVFormatContext *s, AVStream *st, > fmt->name, score); > for (i = 0; fmt_id_type[i].name; i++) { > if (!strcmp(fmt->name, fmt_id_type[i].name)) { > - if (fmt_id_type[i].type != AVMEDIA_TYPE_AUDIO && > - st->codecpar->sample_rate) > + if (st->codecpar->sample_rate) > continue; I believe the correct fix is to change the mp3 probe function so that it does not return a score of 51 for the pcm pattern of ff ff e8 ff ff ff e8 ff... Carl Eugen
On Wed, Nov 6, 2019 at 10:01 AM <lance.lmwang@gmail.com> wrote: > > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavformat/utils.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/libavformat/utils.c b/libavformat/utils.c > index 8196442dd1..8381498e2b 100644 > --- a/libavformat/utils.c > +++ b/libavformat/utils.c > @@ -367,8 +367,7 @@ static int set_codec_from_probe_data(AVFormatContext *s, AVStream *st, > fmt->name, score); > for (i = 0; fmt_id_type[i].name; i++) { > if (!strcmp(fmt->name, fmt_id_type[i].name)) { > - if (fmt_id_type[i].type != AVMEDIA_TYPE_AUDIO && > - st->codecpar->sample_rate) > + if (st->codecpar->sample_rate) > continue; > if (st->request_probe > score && > st->codecpar->codec_id != fmt_id_type[i].id) This change seems wrong, as you are practically rejecting probing of any audio now, since those would get a sample rate. - Hendrik
On Wed, Nov 06, 2019 at 11:18:10AM +0100, Carl Eugen Hoyos wrote: > Am Mi., 6. Nov. 2019 um 10:01 Uhr schrieb <lance.lmwang@gmail.com>: > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavformat/utils.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c > > index 8196442dd1..8381498e2b 100644 > > --- a/libavformat/utils.c > > +++ b/libavformat/utils.c > > @@ -367,8 +367,7 @@ static int set_codec_from_probe_data(AVFormatContext *s, AVStream *st, > > fmt->name, score); > > for (i = 0; fmt_id_type[i].name; i++) { > > if (!strcmp(fmt->name, fmt_id_type[i].name)) { > > - if (fmt_id_type[i].type != AVMEDIA_TYPE_AUDIO && > > - st->codecpar->sample_rate) > > + if (st->codecpar->sample_rate) > > continue; > > I believe the correct fix is to change the mp3 probe function so that it > does not return a score of 51 for the pcm pattern of ff ff e8 ff ff ff e8 ff... It has been detected as pcm and have get the valid audio sample rate, why we detect it again here? I have run fate testing with the patch and haven't find any broken condition. > > Carl Eugen > _______________________________________________ > 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, Nov 06, 2019 at 11:30:41AM +0100, Hendrik Leppkes wrote: > On Wed, Nov 6, 2019 at 10:01 AM <lance.lmwang@gmail.com> wrote: > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavformat/utils.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c > > index 8196442dd1..8381498e2b 100644 > > --- a/libavformat/utils.c > > +++ b/libavformat/utils.c > > @@ -367,8 +367,7 @@ static int set_codec_from_probe_data(AVFormatContext *s, AVStream *st, > > fmt->name, score); > > for (i = 0; fmt_id_type[i].name; i++) { > > if (!strcmp(fmt->name, fmt_id_type[i].name)) { > > - if (fmt_id_type[i].type != AVMEDIA_TYPE_AUDIO && > > - st->codecpar->sample_rate) > > + if (st->codecpar->sample_rate) > > continue; > > if (st->request_probe > score && > > st->codecpar->codec_id != fmt_id_type[i].id) > > This change seems wrong, as you are practically rejecting probing of > any audio now, since those would get a sample rate. By the old logic, I assume sample_rate > 0 means the audio has been detected, so don't probe the data again. The old code is for video only I think. If have condition is broken, it should be add to fate testing. > > - 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".
Am Mi., 6. Nov. 2019 um 12:21 Uhr schrieb Limin Wang <lance.lmwang@gmail.com>: > > On Wed, Nov 06, 2019 at 11:18:10AM +0100, Carl Eugen Hoyos wrote: > > Am Mi., 6. Nov. 2019 um 10:01 Uhr schrieb <lance.lmwang@gmail.com>: > > > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > > --- > > > libavformat/utils.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c > > > index 8196442dd1..8381498e2b 100644 > > > --- a/libavformat/utils.c > > > +++ b/libavformat/utils.c > > > @@ -367,8 +367,7 @@ static int set_codec_from_probe_data(AVFormatContext *s, AVStream *st, > > > fmt->name, score); > > > for (i = 0; fmt_id_type[i].name; i++) { > > > if (!strcmp(fmt->name, fmt_id_type[i].name)) { > > > - if (fmt_id_type[i].type != AVMEDIA_TYPE_AUDIO && > > > - st->codecpar->sample_rate) > > > + if (st->codecpar->sample_rate) > > > continue; > > > > I believe the correct fix is to change the mp3 probe function so that it > > does not return a score of 51 for the pcm pattern of ff ff e8 ff ff ff e8 ff... > > It has been detected as pcm There is no pcm detection in FFmpeg (it is hard iiuc), and the content does not get detected as pcm but mp3 / mp1. > and have get the valid audio sample rate, why we detect it > again here? How else can you rule out dca audio? > I have run fate testing with the patch and haven't find any broken condition. fate cannot test for all possible code paths. Carl Eugen
Am Mi., 6. Nov. 2019 um 12:01 Uhr schrieb Limin Wang <lance.lmwang@gmail.com>: > > On Wed, Nov 06, 2019 at 11:30:41AM +0100, Hendrik Leppkes wrote: > > On Wed, Nov 6, 2019 at 10:01 AM <lance.lmwang@gmail.com> wrote: > > > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > > --- > > > libavformat/utils.c | 3 +-- > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c > > > index 8196442dd1..8381498e2b 100644 > > > --- a/libavformat/utils.c > > > +++ b/libavformat/utils.c > > > @@ -367,8 +367,7 @@ static int set_codec_from_probe_data(AVFormatContext *s, AVStream *st, > > > fmt->name, score); > > > for (i = 0; fmt_id_type[i].name; i++) { > > > if (!strcmp(fmt->name, fmt_id_type[i].name)) { > > > - if (fmt_id_type[i].type != AVMEDIA_TYPE_AUDIO && > > > - st->codecpar->sample_rate) > > > + if (st->codecpar->sample_rate) > > > continue; > > > if (st->request_probe > score && > > > st->codecpar->codec_id != fmt_id_type[i].id) > > > > This change seems wrong, as you are practically rejecting probing of > > any audio now, since those would get a sample rate. > > By the old logic, I assume sample_rate > 0 means the audio has been detected, > so don't probe the data again. If sample_rate > 0 then do not consider non-audio codecs for auto-detection. Carl Eugen
On Wed, Nov 06, 2019 at 08:21:56PM +0100, Carl Eugen Hoyos wrote: > Am Mi., 6. Nov. 2019 um 12:21 Uhr schrieb Limin Wang <lance.lmwang@gmail.com>: > > > > On Wed, Nov 06, 2019 at 11:18:10AM +0100, Carl Eugen Hoyos wrote: > > > Am Mi., 6. Nov. 2019 um 10:01 Uhr schrieb <lance.lmwang@gmail.com>: > > > > > > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > > > --- > > > > libavformat/utils.c | 3 +-- > > > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > > > > > diff --git a/libavformat/utils.c b/libavformat/utils.c > > > > index 8196442dd1..8381498e2b 100644 > > > > --- a/libavformat/utils.c > > > > +++ b/libavformat/utils.c > > > > @@ -367,8 +367,7 @@ static int set_codec_from_probe_data(AVFormatContext *s, AVStream *st, > > > > fmt->name, score); > > > > for (i = 0; fmt_id_type[i].name; i++) { > > > > if (!strcmp(fmt->name, fmt_id_type[i].name)) { > > > > - if (fmt_id_type[i].type != AVMEDIA_TYPE_AUDIO && > > > > - st->codecpar->sample_rate) > > > > + if (st->codecpar->sample_rate) > > > > continue; > > > > > > I believe the correct fix is to change the mp3 probe function so that it > > > does not return a score of 51 for the pcm pattern of ff ff e8 ff ff ff e8 ff... > > > > It has been detected as pcm > > There is no pcm detection in FFmpeg (it is hard iiuc), and the content > does not get detected as pcm but mp3 / mp1. It's detected as wav and get the pcm codec, but if it's AV_CODEC_ID_PCM_S16LE, it'll request probe again, see my updated patch. I add a checking to prove it. > > > and have get the valid audio sample rate, why we detect it > > again here? > > How else can you rule out dca audio? got it, haven't used dca audio before. > > > I have run fate testing with the patch and haven't find any broken condition. > > fate cannot test for all possible code paths. > > Carl Eugen > _______________________________________________ > 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/utils.c b/libavformat/utils.c index 8196442dd1..8381498e2b 100644 --- a/libavformat/utils.c +++ b/libavformat/utils.c @@ -367,8 +367,7 @@ static int set_codec_from_probe_data(AVFormatContext *s, AVStream *st, fmt->name, score); for (i = 0; fmt_id_type[i].name; i++) { if (!strcmp(fmt->name, fmt_id_type[i].name)) { - if (fmt_id_type[i].type != AVMEDIA_TYPE_AUDIO && - st->codecpar->sample_rate) + if (st->codecpar->sample_rate) continue; if (st->request_probe > score && st->codecpar->codec_id != fmt_id_type[i].id)