diff mbox

[FFmpeg-devel] hls: always call avformat_find_stream_info for subdemuxers

Message ID 971a8fc4-5464-8f0e-441d-9c2ce018e2cc@googlemail.com
State Superseded
Headers show

Commit Message

Andreas Cadhalpun Oct. 27, 2016, 7:20 p.m. UTC
This fixes probing dts/eac3/mp2 in hls.

This partly reverts commit 04964ac311abe670fb3b60290a330f2067544b13.

Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun@googlemail.com>
---
 libavformat/hls.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

Comments

Hendrik Leppkes Oct. 27, 2016, 7:30 p.m. UTC | #1
On Thu, Oct 27, 2016 at 9:20 PM, Andreas Cadhalpun
<andreas.cadhalpun@googlemail.com> wrote:
> This fixes probing dts/eac3/mp2 in hls.
>
> This partly reverts commit 04964ac311abe670fb3b60290a330f2067544b13.
>

That commit does a lot of magic flag handling to avoid calling that
function all the time (in fact that seems to be one of the main
purposes of that commit), I don't think a partial revert is a good
idea.
At the very least I would like to hear from the author of that
particular commit.

PS:
AFAIK those codecs are not supported in HLS by the spec, either way.

- Hendrik
Marton Balint Oct. 27, 2016, 8:15 p.m. UTC | #2
On Thu, 27 Oct 2016, Hendrik Leppkes wrote:

> On Thu, Oct 27, 2016 at 9:20 PM, Andreas Cadhalpun
> <andreas.cadhalpun@googlemail.com> wrote:
>> This fixes probing dts/eac3/mp2 in hls.
>>
>> This partly reverts commit 04964ac311abe670fb3b60290a330f2067544b13.
>>
>
> That commit does a lot of magic flag handling to avoid calling that
> function all the time (in fact that seems to be one of the main
> purposes of that commit), I don't think a partial revert is a good
> idea.
> At the very least I would like to hear from the author of that
> particular commit.
>
> PS:
> AFAIK those codecs are not supported in HLS by the spec, either way.

FWIW that particular commit also caused some problems for me, and 
sometimes even AAC audio is detected with 0 channels, I don't really 
understand why.

Regards,
Marton
Andreas Cadhalpun Oct. 27, 2016, 8:31 p.m. UTC | #3
On 27.10.2016 21:30, Hendrik Leppkes wrote:
> On Thu, Oct 27, 2016 at 9:20 PM, Andreas Cadhalpun
> <andreas.cadhalpun@googlemail.com> wrote:
>> This fixes probing dts/eac3/mp2 in hls.
>>
>> This partly reverts commit 04964ac311abe670fb3b60290a330f2067544b13.
>>
> 
> That commit does a lot of magic flag handling to avoid calling that
> function all the time (in fact that seems to be one of the main
> purposes of that commit), I don't think a partial revert is a good
> idea.

I just wanted to keep the change minimal.
Would you prefer a full revert of that commit?

> At the very least I would like to hear from the author of that
> particular commit.

CC'ed the author. Better ideas/patches are always welcome.

> PS:
> AFAIK those codecs are not supported in HLS by the spec, either way.

Maybe, but ffprobe handled them fine in the past, so if that wasn't
intentionally broken, it should be fixed.

Best regards,
Andreas
Andreas Cadhalpun Nov. 3, 2016, 12:14 a.m. UTC | #4
On 27.10.2016 22:31, Andreas Cadhalpun wrote:
> On 27.10.2016 21:30, Hendrik Leppkes wrote:
>> On Thu, Oct 27, 2016 at 9:20 PM, Andreas Cadhalpun
>> <andreas.cadhalpun@googlemail.com> wrote:
>>> This fixes probing dts/eac3/mp2 in hls.
>>>
>>> This partly reverts commit 04964ac311abe670fb3b60290a330f2067544b13.
>>>
>>
>> That commit does a lot of magic flag handling to avoid calling that
>> function all the time (in fact that seems to be one of the main
>> purposes of that commit), I don't think a partial revert is a good
>> idea.
> 
> I just wanted to keep the change minimal.
> Would you prefer a full revert of that commit?
> 
>> At the very least I would like to hear from the author of that
>> particular commit.
> 
> CC'ed the author. Better ideas/patches are always welcome.

I've just sent an alternative patch [1], that keeps the sample from
ticket 4930 working.

Best regards,
Andreas


1: https://ffmpeg.org/pipermail/ffmpeg-devel/2016-November/202165.html
diff mbox

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 3c09dd8..2364144 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1750,17 +1750,9 @@  static int hls_read_header(AVFormatContext *s)
         if (pls->is_id3_timestamped == -1)
             av_log(s, AV_LOG_WARNING, "No expected HTTP requests have been made\n");
 
-        /*
-         * For ID3 timestamped raw audio streams we need to detect the packet
-         * durations to calculate timestamps in fill_timing_for_id3_timestamped_stream(),
-         * but for other streams we can rely on our user calling avformat_find_stream_info()
-         * on us if they want to.
-         */
-        if (pls->is_id3_timestamped) {
-            ret = avformat_find_stream_info(pls->ctx, NULL);
-            if (ret < 0)
-                goto fail;
-        }
+        ret = avformat_find_stream_info(pls->ctx, NULL);
+        if (ret < 0)
+            goto fail;
 
         pls->has_noheader_flag = !!(pls->ctx->ctx_flags & AVFMTCTX_NOHEADER);