diff mbox

[FFmpeg-devel,2/2] avformat/hls: Fix probing mpegts audio streams that use probing

Message ID 1478363940-21822-3-git-send-email-anssi.hannula@iki.fi
State Superseded
Headers show

Commit Message

Anssi Hannula Nov. 5, 2016, 4:39 p.m. UTC 
Commit 04964ac311abe670f ("avformat/hls: Fix missing streams in some
cases with MPEG TS") caused a regression where subdemuxer streams that
use probing (e.g. dts/eac3/mp2 in mpegts) no longer get probed properly.

This is because the codec parameters from the subdemuxer stream, once
probed, are not passed on to the main stream.

Fix that by updating the codec parameters if the codec id changes.

Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
---
 libavformat/hls.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Hendrik Leppkes Nov. 5, 2016, 5:27 p.m. UTC | #1 
On Sat, Nov 5, 2016 at 5:39 PM, Anssi Hannula <anssi.hannula@iki.fi> wrote:
> Commit 04964ac311abe670f ("avformat/hls: Fix missing streams in some
> cases with MPEG TS") caused a regression where subdemuxer streams that
> use probing (e.g. dts/eac3/mp2 in mpegts) no longer get probed properly.
>
> This is because the codec parameters from the subdemuxer stream, once
> probed, are not passed on to the main stream.
>
> Fix that by updating the codec parameters if the codec id changes.
>
> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
> ---
>  libavformat/hls.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 6fb652c..8527f33 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -1950,6 +1950,8 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>      /* If we got a packet, return it */
>      if (minplaylist >= 0) {
>          struct playlist *pls = c->playlists[minplaylist];
> +        AVStream *ist;
> +        AVStream *st;
>
>          ret = update_streams_from_subdemuxer(s, pls);
>          if (ret < 0) {
> @@ -1972,8 +1974,11 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>              return AVERROR_BUG;
>          }
>
> +        ist = pls->ctx->streams[pls->pkt.stream_index];
> +        st = pls->main_streams[pls->pkt.stream_index];
> +
>          *pkt = pls->pkt;
> -        pkt->stream_index = pls->main_streams[pls->pkt.stream_index]->index;
> +        pkt->stream_index = st->index;
>          reset_packet(&c->playlists[minplaylist]->pkt);
>
>          if (pkt->dts != AV_NOPTS_VALUE)
> @@ -1981,6 +1986,11 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>                                              pls->ctx->streams[pls->pkt.stream_index]->time_base,
>                                              AV_TIME_BASE_Q);
>
> +        /* There may be more situations where this would be useful, but this at least
> +         * handles newly probed codecs properly (i.e. request_probe by mpegts). */
> +        if (ist->codecpar->codec_id != st->codecpar->codec_id)
> +            set_stream_info_from_input_stream(st, pls, ist);
> +

A better solution would be checking
ist->internal->need_context_update, and also setting this in the
output stream when you copy params over, so the generic code knows
things changed.

- Hendrik
Andreas Cadhalpun Nov. 5, 2016, 5:47 p.m. UTC | #2 
On 05.11.2016 17:39, Anssi Hannula wrote:
> Commit 04964ac311abe670f ("avformat/hls: Fix missing streams in some
> cases with MPEG TS") caused a regression where subdemuxer streams that
> use probing (e.g. dts/eac3/mp2 in mpegts) no longer get probed properly.
> 
> This is because the codec parameters from the subdemuxer stream, once
> probed, are not passed on to the main stream.
> 
> Fix that by updating the codec parameters if the codec id changes.
> 
> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
> ---
>  libavformat/hls.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 6fb652c..8527f33 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -1950,6 +1950,8 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>      /* If we got a packet, return it */
>      if (minplaylist >= 0) {
>          struct playlist *pls = c->playlists[minplaylist];
> +        AVStream *ist;
> +        AVStream *st;
>  
>          ret = update_streams_from_subdemuxer(s, pls);
>          if (ret < 0) {
> @@ -1972,8 +1974,11 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>              return AVERROR_BUG;
>          }
>  
> +        ist = pls->ctx->streams[pls->pkt.stream_index];
> +        st = pls->main_streams[pls->pkt.stream_index];
> +
>          *pkt = pls->pkt;
> -        pkt->stream_index = pls->main_streams[pls->pkt.stream_index]->index;
> +        pkt->stream_index = st->index;
>          reset_packet(&c->playlists[minplaylist]->pkt);
>  
>          if (pkt->dts != AV_NOPTS_VALUE)
> @@ -1981,6 +1986,11 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>                                              pls->ctx->streams[pls->pkt.stream_index]->time_base,
>                                              AV_TIME_BASE_Q);
>  
> +        /* There may be more situations where this would be useful, but this at least
> +         * handles newly probed codecs properly (i.e. request_probe by mpegts). */
> +        if (ist->codecpar->codec_id != st->codecpar->codec_id)
> +            set_stream_info_from_input_stream(st, pls, ist);

This has to set:
            ist->internal->need_context_update = 1;

Otherwise mp2 is still detected as mp3.

Best regards,
Andreas
Andreas Cadhalpun Nov. 5, 2016, 5:51 p.m. UTC | #3 
On 05.11.2016 18:47, Andreas Cadhalpun wrote:
> On 05.11.2016 17:39, Anssi Hannula wrote:
>> @@ -1981,6 +1986,11 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>>                                              pls->ctx->streams[pls->pkt.stream_index]->time_base,
>>                                              AV_TIME_BASE_Q);
>>  
>> +        /* There may be more situations where this would be useful, but this at least
>> +         * handles newly probed codecs properly (i.e. request_probe by mpegts). */
>> +        if (ist->codecpar->codec_id != st->codecpar->codec_id)
>> +            set_stream_info_from_input_stream(st, pls, ist);
> 
> This has to set:
>             ist->internal->need_context_update = 1;
              ^
Should have been 'st' not 'ist'.

Best regards,
Andreas
Anssi Hannula Nov. 6, 2016, 2:04 p.m. UTC | #4 
05.11.2016, 19:27, Hendrik Leppkes kirjoitti:
> On Sat, Nov 5, 2016 at 5:39 PM, Anssi Hannula <anssi.hannula@iki.fi> wrote:
>> Commit 04964ac311abe670f ("avformat/hls: Fix missing streams in some
>> cases with MPEG TS") caused a regression where subdemuxer streams that
>> use probing (e.g. dts/eac3/mp2 in mpegts) no longer get probed properly.
>>
>> This is because the codec parameters from the subdemuxer stream, once
>> probed, are not passed on to the main stream.
>>
>> Fix that by updating the codec parameters if the codec id changes.
>>
>> Signed-off-by: Anssi Hannula <anssi.hannula@iki.fi>
>> ---
>>  libavformat/hls.c | 12 +++++++++++-
>>  1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>> index 6fb652c..8527f33 100644
>> --- a/libavformat/hls.c
>> +++ b/libavformat/hls.c
>> @@ -1950,6 +1950,8 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>>      /* If we got a packet, return it */
>>      if (minplaylist >= 0) {
>>          struct playlist *pls = c->playlists[minplaylist];
>> +        AVStream *ist;
>> +        AVStream *st;
>>
>>          ret = update_streams_from_subdemuxer(s, pls);
>>          if (ret < 0) {
>> @@ -1972,8 +1974,11 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>>              return AVERROR_BUG;
>>          }
>>
>> +        ist = pls->ctx->streams[pls->pkt.stream_index];
>> +        st = pls->main_streams[pls->pkt.stream_index];
>> +
>>          *pkt = pls->pkt;
>> -        pkt->stream_index = pls->main_streams[pls->pkt.stream_index]->index;
>> +        pkt->stream_index = st->index;
>>          reset_packet(&c->playlists[minplaylist]->pkt);
>>
>>          if (pkt->dts != AV_NOPTS_VALUE)
>> @@ -1981,6 +1986,11 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>>                                              pls->ctx->streams[pls->pkt.stream_index]->time_base,
>>                                              AV_TIME_BASE_Q);
>>
>> +        /* There may be more situations where this would be useful, but this at least
>> +         * handles newly probed codecs properly (i.e. request_probe by mpegts). */
>> +        if (ist->codecpar->codec_id != st->codecpar->codec_id)
>> +            set_stream_info_from_input_stream(st, pls, ist);
>> +
> 
> A better solution would be checking
> ist->internal->need_context_update, and also setting this in the
> output stream when you copy params over, so the generic code knows
> things changed.

AFAICS that flag has already been handled & cleared by
read_frame_internal() at this point.

So I'll do what Andreas suggested and just set it to 1 here (or in
set_stream_in_from_input_stream), unless I'm missing something...
Anssi Hannula Nov. 6, 2016, 9:44 p.m. UTC | #5 
05.11.2016, 19:51, Andreas Cadhalpun kirjoitti:
> On 05.11.2016 18:47, Andreas Cadhalpun wrote:
>> On 05.11.2016 17:39, Anssi Hannula wrote:
>>> @@ -1981,6 +1986,11 @@ static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>>>                                              pls->ctx->streams[pls->pkt.stream_index]->time_base,
>>>                                              AV_TIME_BASE_Q);
>>>  
>>> +        /* There may be more situations where this would be useful, but this at least
>>> +         * handles newly probed codecs properly (i.e. request_probe by mpegts). */
>>> +        if (ist->codecpar->codec_id != st->codecpar->codec_id)
>>> +            set_stream_info_from_input_stream(st, pls, ist);
>>
>> This has to set:
>>             ist->internal->need_context_update = 1;
>               ^
> Should have been 'st' not 'ist'.

Thanks for the comments, followup is a new set with
need_context_update = 1 added to set_stream_info_from_input_stream() and
an additional patch to add error checks for the use of
avcodec_parameters_copy().


Anssi Hannula (3):
      avformat/hls: Factor copying stream info to a separate function
      avformat/hls: Fix probing mpegts audio streams that use probing
      avformat/hls: Add missing error check for avcodec_parameters_copy()
diff mbox

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 6fb652c..8527f33 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1950,6 +1950,8 @@  static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
     /* If we got a packet, return it */
     if (minplaylist >= 0) {
         struct playlist *pls = c->playlists[minplaylist];
+        AVStream *ist;
+        AVStream *st;
 
         ret = update_streams_from_subdemuxer(s, pls);
         if (ret < 0) {
@@ -1972,8 +1974,11 @@  static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
             return AVERROR_BUG;
         }
 
+        ist = pls->ctx->streams[pls->pkt.stream_index];
+        st = pls->main_streams[pls->pkt.stream_index];
+
         *pkt = pls->pkt;
-        pkt->stream_index = pls->main_streams[pls->pkt.stream_index]->index;
+        pkt->stream_index = st->index;
         reset_packet(&c->playlists[minplaylist]->pkt);
 
         if (pkt->dts != AV_NOPTS_VALUE)
@@ -1981,6 +1986,11 @@  static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
                                             pls->ctx->streams[pls->pkt.stream_index]->time_base,
                                             AV_TIME_BASE_Q);
 
+        /* There may be more situations where this would be useful, but this at least
+         * handles newly probed codecs properly (i.e. request_probe by mpegts). */
+        if (ist->codecpar->codec_id != st->codecpar->codec_id)
+            set_stream_info_from_input_stream(st, pls, ist);
+
         return 0;
     }
     return AVERROR_EOF;