| Message ID | 20230515000547.1703-3-michael@niedermayer.cc |
|---|---|
| State | Accepted |
| Commit | 954d16fa3f09a04c7917a1c69a5c3e283554cb1d |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avformat/hls: reduce default max reload to 3 | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On Mon, May 15, 2023 at 02:05:47AM +0200, Michael Niedermayer wrote: > This is not well tested and can likely be improved, just a > hotfix for hls probe failures since 6b1f68ccb04d791f0250e05687c346a99ff47ea1 > > Should fix Ticket10353 (please test and report cases that still fail) > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/hls.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) i will apply this real soon as it seems to affect many people [...]
On Mon, May 15, 2023 at 02:17:35AM +0200, Michael Niedermayer wrote: > On Mon, May 15, 2023 at 02:05:47AM +0200, Michael Niedermayer wrote: > > This is not well tested and can likely be improved, just a > > hotfix for hls probe failures since 6b1f68ccb04d791f0250e05687c346a99ff47ea1 > > > > Should fix Ticket10353 (please test and report cases that still fail) > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/hls.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > i will apply this real soon as it seems to affect many people Patch 2 & 3 applied [...]
On 5/14/23 20:05, Michael Niedermayer wrote: > This is not well tested and can likely be improved, just a > hotfix for hls probe failures since 6b1f68ccb04d791f0250e05687c346a99ff47ea1 > > Should fix Ticket10353 (please test and report cases that still fail) > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/hls.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index df2442c376..790ae7a96a 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -2534,7 +2534,16 @@ static int hls_probe(const AVProbeData *p) > strstr(p->buf, "#EXT-X-TARGETDURATION:") || > strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:")) { > > - if (!av_match_ext(p->filename, "m3u8,hls,m3u")) { > + int mime_ok = p->mime_type && !( > + av_strcasecmp(p->mime_type, "application/vnd.apple.mpegurl") && > + av_strcasecmp(p->mime_type, "audio/mpegurl") && > + av_strcasecmp(p->mime_type, "audio/x-mpegurl") && > + av_strcasecmp(p->mime_type, "application/x-mpegurl") > + ); > + > + if (!av_match_ext (p->filename, "m3u8,hls,m3u") && > + ff_match_url_ext(p->filename, "m3u8,hls,m3u") <= 0 && What's the point of checking both av_match_ext and ff_match_url_ext here? Should only want one or the other. > + !mime_ok) { Put !mime_ok first to take advantage of lazy &&, as it will usually be okay in practice. > av_log(NULL, AV_LOG_ERROR, "Not detecting m3u8/hls with non standard extension\n"); > return 0; > }
On Mon, 15 May 2023 at 02:06, Michael Niedermayer <michael@niedermayer.cc> wrote: > > This is not well tested and can likely be improved, just a > hotfix for hls probe failures since 6b1f68ccb04d791f0250e05687c346a99ff47ea1 > > Should fix Ticket10353 (please test and report cases that still fail) > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavformat/hls.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/libavformat/hls.c b/libavformat/hls.c > index df2442c376..790ae7a96a 100644 > --- a/libavformat/hls.c > +++ b/libavformat/hls.c > @@ -2534,7 +2534,16 @@ static int hls_probe(const AVProbeData *p) > strstr(p->buf, "#EXT-X-TARGETDURATION:") || > strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:")) { > > - if (!av_match_ext(p->filename, "m3u8,hls,m3u")) { > + int mime_ok = p->mime_type && !( > + av_strcasecmp(p->mime_type, "application/vnd.apple.mpegurl") && > + av_strcasecmp(p->mime_type, "audio/mpegurl") && > + av_strcasecmp(p->mime_type, "audio/x-mpegurl") && > + av_strcasecmp(p->mime_type, "application/x-mpegurl") How about we AV_LOG_WARNING when non-standard/deprecated mime type is used? If we want to be strict about rfc8216 only two first should be supported. Of course for compatibility reasons likely we need to support all of them, but warn about it would be nice touch. > + ); > + > + if (!av_match_ext (p->filename, "m3u8,hls,m3u") && > + ff_match_url_ext(p->filename, "m3u8,hls,m3u") <= 0 && Where '.hls' came from? I don't think those are in fact used in the wild? Maybe we can be strict and use only "m3u8,m3u"? > + !mime_ok) { > av_log(NULL, AV_LOG_ERROR, "Not detecting m3u8/hls with non standard extension\n"); This log wording is little bit off now, when there is no extension and only mime matching. > return 0; > } > -- > 2.17.1 > > _______________________________________________ > 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". Minor remarks, but funcionally the patch resolves the issue. Thanks. - Kacper
On Sun, May 14, 2023 at 11:14:12PM -0400, Leo Izen wrote: > > > On 5/14/23 20:05, Michael Niedermayer wrote: > > This is not well tested and can likely be improved, just a > > hotfix for hls probe failures since 6b1f68ccb04d791f0250e05687c346a99ff47ea1 > > > > Should fix Ticket10353 (please test and report cases that still fail) > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/hls.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/hls.c b/libavformat/hls.c > > index df2442c376..790ae7a96a 100644 > > --- a/libavformat/hls.c > > +++ b/libavformat/hls.c > > @@ -2534,7 +2534,16 @@ static int hls_probe(const AVProbeData *p) > > strstr(p->buf, "#EXT-X-TARGETDURATION:") || > > strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:")) { > > - if (!av_match_ext(p->filename, "m3u8,hls,m3u")) { > > + int mime_ok = p->mime_type && !( > > + av_strcasecmp(p->mime_type, "application/vnd.apple.mpegurl") && > > + av_strcasecmp(p->mime_type, "audio/mpegurl") && > > + av_strcasecmp(p->mime_type, "audio/x-mpegurl") && > > + av_strcasecmp(p->mime_type, "application/x-mpegurl") > > + ); > > + > > + if (!av_match_ext (p->filename, "m3u8,hls,m3u") && > > + ff_match_url_ext(p->filename, "m3u8,hls,m3u") <= 0 && > What's the point of checking both av_match_ext and ff_match_url_ext here? > Should only want one or the other. is p->filename a URL ? > > + !mime_ok) { > > Put !mime_ok first to take advantage of lazy &&, as it will usually be okay > in practice. ok, will reorder it thx [...]
On Mon, May 15, 2023 at 11:58:16AM +0200, Kacper Michajlow wrote: > On Mon, 15 May 2023 at 02:06, Michael Niedermayer > <michael@niedermayer.cc> wrote: > > > > This is not well tested and can likely be improved, just a > > hotfix for hls probe failures since 6b1f68ccb04d791f0250e05687c346a99ff47ea1 > > > > Should fix Ticket10353 (please test and report cases that still fail) > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavformat/hls.c | 11 ++++++++++- > > 1 file changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/libavformat/hls.c b/libavformat/hls.c > > index df2442c376..790ae7a96a 100644 > > --- a/libavformat/hls.c > > +++ b/libavformat/hls.c > > @@ -2534,7 +2534,16 @@ static int hls_probe(const AVProbeData *p) > > strstr(p->buf, "#EXT-X-TARGETDURATION:") || > > strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:")) { > > > > - if (!av_match_ext(p->filename, "m3u8,hls,m3u")) { > > + int mime_ok = p->mime_type && !( > > + av_strcasecmp(p->mime_type, "application/vnd.apple.mpegurl") && > > + av_strcasecmp(p->mime_type, "audio/mpegurl") && > > + av_strcasecmp(p->mime_type, "audio/x-mpegurl") && > > + av_strcasecmp(p->mime_type, "application/x-mpegurl") > > How about we AV_LOG_WARNING when non-standard/deprecated mime type is > used? If we want to be strict about rfc8216 only two first should be > supported. Of course for compatibility reasons likely we need to > support all of them, but warn about it would be nice touch. ok > > > + ); > > + > > + if (!av_match_ext (p->filename, "m3u8,hls,m3u") && > > + ff_match_url_ext(p->filename, "m3u8,hls,m3u") <= 0 && > > Where '.hls' came from? I don't think those are in fact used in the > wild? Maybe we can be strict and use only "m3u8,m3u"? i was thinking of it but i left them in yesterday. Ill post a patch that would remove them. I have a few files ending in hls but none of them qualify as "in the wild" > > > + !mime_ok) { > > av_log(NULL, AV_LOG_ERROR, "Not detecting m3u8/hls with non standard extension\n"); > > This log wording is little bit off now, when there is no extension and > only mime matching. will post a patch thx [...]
diff --git a/libavformat/hls.c b/libavformat/hls.c index df2442c376..790ae7a96a 100644 --- a/libavformat/hls.c +++ b/libavformat/hls.c @@ -2534,7 +2534,16 @@ static int hls_probe(const AVProbeData *p) strstr(p->buf, "#EXT-X-TARGETDURATION:") || strstr(p->buf, "#EXT-X-MEDIA-SEQUENCE:")) { - if (!av_match_ext(p->filename, "m3u8,hls,m3u")) { + int mime_ok = p->mime_type && !( + av_strcasecmp(p->mime_type, "application/vnd.apple.mpegurl") && + av_strcasecmp(p->mime_type, "audio/mpegurl") && + av_strcasecmp(p->mime_type, "audio/x-mpegurl") && + av_strcasecmp(p->mime_type, "application/x-mpegurl") + ); + + if (!av_match_ext (p->filename, "m3u8,hls,m3u") && + ff_match_url_ext(p->filename, "m3u8,hls,m3u") <= 0 && + !mime_ok) { av_log(NULL, AV_LOG_ERROR, "Not detecting m3u8/hls with non standard extension\n"); return 0; }
This is not well tested and can likely be improved, just a hotfix for hls probe failures since 6b1f68ccb04d791f0250e05687c346a99ff47ea1 Should fix Ticket10353 (please test and report cases that still fail) Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavformat/hls.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)