[1/2] libavformat: Don't require a codec to be set for data streams

Message ID 1333721603-66147-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö April 6, 2012, 2:13 p.m.
This avoids blocking for a long time in
avformat_find_stream_info while waiting for a codec to be set
for data streams in MS-RTSP streams, where the codec won't
ever be set.

This is an adaptation of one part of commit 4c050429780b0d
in FFmpeg.

Alternatively, we could just ignore data streams in MS-RTSP
and not pass them through to the caller.
---
 libavformat/utils.c |    2 ++
 1 file changed, 2 insertions(+)

Comments

Ronald Bultje April 6, 2012, 2:32 p.m. | #1
Hi,

On Fri, Apr 6, 2012 at 7:13 AM, Martin Storsjö <martin@martin.st> wrote:
> This avoids blocking for a long time in
> avformat_find_stream_info while waiting for a codec to be set
> for data streams in MS-RTSP streams, where the codec won't
> ever be set.
>
> This is an adaptation of one part of commit 4c050429780b0d
> in FFmpeg.
>
> Alternatively, we could just ignore data streams in MS-RTSP
> and not pass them through to the caller.
> ---
>  libavformat/utils.c |    2 ++
>  1 file changed, 2 insertions(+)

I'd say your suggestion sounds better. A codec-less stream is by
definition useless. Why do we pass data streams? Do they ever contain
actual relevant data? (I don't remember anymore.) If not, they
shouldn't be user-visible.

Ronald
Martin Storsjö April 6, 2012, 3:29 p.m. | #2
On Fri, 6 Apr 2012, Ronald S. Bultje wrote:

> Hi,
>
> On Fri, Apr 6, 2012 at 7:13 AM, Martin Storsjö <martin@martin.st> wrote:
>> This avoids blocking for a long time in
>> avformat_find_stream_info while waiting for a codec to be set
>> for data streams in MS-RTSP streams, where the codec won't
>> ever be set.
>>
>> This is an adaptation of one part of commit 4c050429780b0d
>> in FFmpeg.
>>
>> Alternatively, we could just ignore data streams in MS-RTSP
>> and not pass them through to the caller.
>> ---
>>  libavformat/utils.c |    2 ++
>>  1 file changed, 2 insertions(+)
>
> I'd say your suggestion sounds better. A codec-less stream is by
> definition useless. Why do we pass data streams? Do they ever contain
> actual relevant data? (I don't remember anymore.) If not, they
> shouldn't be user-visible.

I'm not sure if they contain anything of value.

It seems it's not totally trivial to do this, as streams are added while 
parsing the SDP. If a section of it looks like this:

m=application 0 RTP/AVP 96
b=RS:0
b=RR:0
a=rtpmap:96 x-wms-rtx/1000
a=control:rtx
a=stream:65536

When parsing the m=application line, we add the AVStream (unless it's not 
part of the media_type_mask). Only once we parse rtpmap, we realize it's 
not a known codec - if we don't want that exposed to the caller, we'd have 
to remove the AVStream again - not as straightforward as one would like.

Another option would be to remove AVMEDIA_TYPE_DATA from the default 
media_type_mask, but mpeg2ts in rtp is signalled as that data type (that's 
about the only case where we actually do anything with a m=application 
stream in SDP at the moment).

// Martin
Luca Barbato April 6, 2012, 5:06 p.m. | #3
On 06/04/12 07:13, Martin Storsjö wrote:
> This avoids blocking for a long time in
> avformat_find_stream_info while waiting for a codec to be set
> for data streams in MS-RTSP streams, where the codec won't
> ever be set.
> 
> This is an adaptation of one part of commit 4c050429780b0d
> in FFmpeg.
> 
> Alternatively, we could just ignore data streams in MS-RTSP
> and not pass them through to the caller.
> ---
>  libavformat/utils.c |    2 ++
>  1 file changed, 2 insertions(+)
> 

Seems fine, do we have other data stream beside this case an my flv/rtmp
one?

lu
Luca Barbato April 6, 2012, 5:10 p.m. | #4
On 06/04/12 07:32, Ronald S. Bultje wrote:
> I'd say your suggestion sounds better. A codec-less stream is by
> definition useless. Why do we pass data streams? Do they ever contain
> actual relevant data? (I don't remember anymore.) If not, they
> shouldn't be user-visible.

another possibility is to make the data stream have a default codec, we
are talking about opaque streams that might have a meaning for the user
but not for us.

lu
Ronald Bultje April 6, 2012, 5:39 p.m. | #5
Hi,

On Fri, Apr 6, 2012 at 8:29 AM, Martin Storsjö <martin@martin.st> wrote:
> On Fri, 6 Apr 2012, Ronald S. Bultje wrote:
>> On Fri, Apr 6, 2012 at 7:13 AM, Martin Storsjö <martin@martin.st> wrote:
>>>
>>> This avoids blocking for a long time in
>>> avformat_find_stream_info while waiting for a codec to be set
>>> for data streams in MS-RTSP streams, where the codec won't
>>> ever be set.
>>>
>>> This is an adaptation of one part of commit 4c050429780b0d
>>> in FFmpeg.
>>>
>>> Alternatively, we could just ignore data streams in MS-RTSP
>>> and not pass them through to the caller.
>>> ---
>>>  libavformat/utils.c |    2 ++
>>>  1 file changed, 2 insertions(+)
>>
>>
>> I'd say your suggestion sounds better. A codec-less stream is by
>> definition useless. Why do we pass data streams? Do they ever contain
>> actual relevant data? (I don't remember anymore.) If not, they
>> shouldn't be user-visible.
>
>
> I'm not sure if they contain anything of value.
>
> It seems it's not totally trivial to do this, as streams are added while
> parsing the SDP. If a section of it looks like this:
>
> m=application 0 RTP/AVP 96
> b=RS:0
> b=RR:0
> a=rtpmap:96 x-wms-rtx/1000
> a=control:rtx
> a=stream:65536
>
> When parsing the m=application line, we add the AVStream (unless it's not
> part of the media_type_mask). Only once we parse rtpmap, we realize it's not
> a known codec - if we don't want that exposed to the caller, we'd have to
> remove the AVStream again - not as straightforward as one would like.
>
> Another option would be to remove AVMEDIA_TYPE_DATA from the default
> media_type_mask, but mpeg2ts in rtp is signalled as that data type (that's
> about the only case where we actually do anything with a m=application
> stream in SDP at the moment).

This may well be why I exposed the stream int he first place (i.e.
laziness). We should simply refactor that code so it doesn't add the
stream until we know we should.

Ronald

Patch

diff --git a/libavformat/utils.c b/libavformat/utils.c
index f38045c..c657f97 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -2061,6 +2061,8 @@  static int has_codec_parameters(AVStream *st)
         if (st->info->found_decoder >= 0 && avctx->pix_fmt == PIX_FMT_NONE)
             return 0;
         break;
+    case AVMEDIA_TYPE_DATA:
+        return 1;
     default:
         val = 1;
         break;