[1/2] asfdec: Add an option for not searching for the packet markers
diff mbox

Message ID 1333720983-65603-1-git-send-email-martin@martin.st
State Committed
Commit 75b7feaeb488e8bfa8ce9ba9958c0c6fa704f2ad
Headers show

Commit Message

Martin Storsjö April 6, 2012, 2:03 p.m. 
Some streams don't contain these.
---
I also tried changing the code in ff_asf_get_packet to seek back
and try to restart with the assumption that there isn't any start
marker (the code with a FIXME remark), but it broke in various
odd cases, this seems to be a more straightforward solution
which shouldn't break the existing error tolerance/resync code.

 libavformat/asfdec.c |   21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

Comments

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

On Fri, Apr 6, 2012 at 7:03 AM, Martin Storsjö <martin@martin.st> wrote:
> Some streams don't contain these.
> ---
> I also tried changing the code in ff_asf_get_packet to seek back
> and try to restart with the assumption that there isn't any start
> marker (the code with a FIXME remark), but it broke in various
> odd cases, this seems to be a more straightforward solution
> which shouldn't break the existing error tolerance/resync code.
>
>  libavformat/asfdec.c |   21 ++++++++++++++++++++-
>  1 file changed, 20 insertions(+), 1 deletion(-)

Example of such a stream?

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

> On Fri, Apr 6, 2012 at 7:03 AM, Martin Storsjö <martin@martin.st> wrote:
>> Some streams don't contain these.
>> ---
>> I also tried changing the code in ff_asf_get_packet to seek back
>> and try to restart with the assumption that there isn't any start
>> marker (the code with a FIXME remark), but it broke in various
>> odd cases, this seems to be a more straightforward solution
>> which shouldn't break the existing error tolerance/resync code.
>>
>>  libavformat/asfdec.c |   21 ++++++++++++++++++++-
>>  1 file changed, 20 insertions(+), 1 deletion(-)
>
> Example of such a stream?

rtsp://208.86.19.32:8000/crtn is such a stream.

// Martin
Ronald Bultje April 6, 2012, 9:45 p.m. | #3 
Hi,

On Fri, Apr 6, 2012 at 7:57 AM, Martin Storsjö <martin@martin.st> wrote:
> On Fri, 6 Apr 2012, Ronald S. Bultje wrote:
>> On Fri, Apr 6, 2012 at 7:03 AM, Martin Storsjö <martin@martin.st> wrote:
>>>
>>> Some streams don't contain these.
>>> ---
>>> I also tried changing the code in ff_asf_get_packet to seek back
>>> and try to restart with the assumption that there isn't any start
>>> marker (the code with a FIXME remark), but it broke in various
>>> odd cases, this seems to be a more straightforward solution
>>> which shouldn't break the existing error tolerance/resync code.
>>>
>>>  libavformat/asfdec.c |   21 ++++++++++++++++++++-
>>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>
>>
>> Example of such a stream?
>
>
> rtsp://208.86.19.32:8000/crtn is such a stream.

I remember from when I wrote this stuff that in general, ASF-in-RTSP
doesn't need these markers at all anyway, and each RPT packet simply
contains an ASF packet. My original implementation skipped that
altogether and directly called parse_packet() instead of get_packet().
Maybe we should go back to that approach by default in ASF/RTSP?

Ronald
Ronald Bultje April 6, 2012, 9:46 p.m. | #4 
Hi,

On Fri, Apr 6, 2012 at 2:45 PM, Ronald S. Bultje <rsbultje@gmail.com> wrote:
> Hi,
>
> On Fri, Apr 6, 2012 at 7:57 AM, Martin Storsjö <martin@martin.st> wrote:
>> On Fri, 6 Apr 2012, Ronald S. Bultje wrote:
>>> On Fri, Apr 6, 2012 at 7:03 AM, Martin Storsjö <martin@martin.st> wrote:
>>>>
>>>> Some streams don't contain these.
>>>> ---
>>>> I also tried changing the code in ff_asf_get_packet to seek back
>>>> and try to restart with the assumption that there isn't any start
>>>> marker (the code with a FIXME remark), but it broke in various
>>>> odd cases, this seems to be a more straightforward solution
>>>> which shouldn't break the existing error tolerance/resync code.
>>>>
>>>>  libavformat/asfdec.c |   21 ++++++++++++++++++++-
>>>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>>
>>>
>>> Example of such a stream?
>>
>>
>> rtsp://208.86.19.32:8000/crtn is such a stream.
>
> I remember from when I wrote this stuff that in general, ASF-in-RTSP
> doesn't need these markers at all anyway, and each RPT packet simply
> contains an ASF packet. My original implementation skipped that
> altogether and directly called parse_packet() instead of get_packet().
> Maybe we should go back to that approach by default in ASF/RTSP?

Behaviour, not approach, i.e. set this option to TRUE by default from
the RTP/ASF parser.

Ronald
Martin Storsjö April 6, 2012, 9:56 p.m. | #5 
On Fri, 6 Apr 2012, Ronald S. Bultje wrote:

> Hi,
>
> On Fri, Apr 6, 2012 at 2:45 PM, Ronald S. Bultje <rsbultje@gmail.com> wrote:
>> Hi,
>>
>> On Fri, Apr 6, 2012 at 7:57 AM, Martin Storsjö <martin@martin.st> wrote:
>>> On Fri, 6 Apr 2012, Ronald S. Bultje wrote:
>>>> On Fri, Apr 6, 2012 at 7:03 AM, Martin Storsjö <martin@martin.st> wrote:
>>>>>
>>>>> Some streams don't contain these.
>>>>> ---
>>>>> I also tried changing the code in ff_asf_get_packet to seek back
>>>>> and try to restart with the assumption that there isn't any start
>>>>> marker (the code with a FIXME remark), but it broke in various
>>>>> odd cases, this seems to be a more straightforward solution
>>>>> which shouldn't break the existing error tolerance/resync code.
>>>>>
>>>>>  libavformat/asfdec.c |   21 ++++++++++++++++++++-
>>>>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>>>
>>>>
>>>> Example of such a stream?
>>>
>>>
>>> rtsp://208.86.19.32:8000/crtn is such a stream.
>>
>> I remember from when I wrote this stuff that in general, ASF-in-RTSP
>> doesn't need these markers at all anyway, and each RPT packet simply
>> contains an ASF packet. My original implementation skipped that
>> altogether and directly called parse_packet() instead of get_packet().
>> Maybe we should go back to that approach by default in ASF/RTSP?
>
> Behaviour, not approach, i.e. set this option to TRUE by default from
> the RTP/ASF parser.

Yeah, that's exactly what I do in patch 2/2 in this series.

// Martin
Ronald Bultje April 6, 2012, 9:58 p.m. | #6 
Hi,

On Fri, Apr 6, 2012 at 2:56 PM, Martin Storsjö <martin@martin.st> wrote:
> On Fri, 6 Apr 2012, Ronald S. Bultje wrote:
>> On Fri, Apr 6, 2012 at 2:45 PM, Ronald S. Bultje <rsbultje@gmail.com>
>> wrote:
>>> On Fri, Apr 6, 2012 at 7:57 AM, Martin Storsjö <martin@martin.st> wrote:
>>>> On Fri, 6 Apr 2012, Ronald S. Bultje wrote:
>>>>> On Fri, Apr 6, 2012 at 7:03 AM, Martin Storsjö <martin@martin.st>
>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> Some streams don't contain these.
>>>>>> ---
>>>>>> I also tried changing the code in ff_asf_get_packet to seek back
>>>>>> and try to restart with the assumption that there isn't any start
>>>>>> marker (the code with a FIXME remark), but it broke in various
>>>>>> odd cases, this seems to be a more straightforward solution
>>>>>> which shouldn't break the existing error tolerance/resync code.
>>>>>>
>>>>>>  libavformat/asfdec.c |   21 ++++++++++++++++++++-
>>>>>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>>>>
>>>>>
>>>>>
>>>>> Example of such a stream?
>>>>
>>>>
>>>>
>>>> rtsp://208.86.19.32:8000/crtn is such a stream.
>>>
>>>
>>> I remember from when I wrote this stuff that in general, ASF-in-RTSP
>>> doesn't need these markers at all anyway, and each RPT packet simply
>>> contains an ASF packet. My original implementation skipped that
>>> altogether and directly called parse_packet() instead of get_packet().
>>> Maybe we should go back to that approach by default in ASF/RTSP?
>>
>>
>> Behaviour, not approach, i.e. set this option to TRUE by default from
>> the RTP/ASF parser.
>
>
> Yeah, that's exactly what I do in patch 2/2 in this series.

OMG I feel to stupid now. Patch (both) OK.

Ronald

Patch
diff mbox 

diff --git a/libavformat/asfdec.c b/libavformat/asfdec.c
index 0cb43a5..b39d2f2 100644
--- a/libavformat/asfdec.c
+++ b/libavformat/asfdec.c
@@ -26,6 +26,7 @@ 
 #include "libavutil/avstring.h"
 #include "libavutil/dict.h"
 #include "libavutil/mathematics.h"
+#include "libavutil/opt.h"
 #include "avformat.h"
 #include "internal.h"
 #include "avio_internal.h"
@@ -35,6 +36,7 @@ 
 #include "avlanguage.h"
 
 typedef struct {
+    const AVClass *class;
     int asfid2avid[128];                 ///< conversion table from asf ID 2 AVStream ID
     ASFStream streams[128];              ///< it's max number and it's not that big
     uint32_t stream_bitrates[128];       ///< max number of streams, bitrate for each (for streaming)
@@ -72,8 +74,22 @@  typedef struct {
     int stream_index;
 
     ASFStream* asf_st;                   ///< currently decoded stream
+
+    int no_resync_search;
 } ASFContext;
 
+static const AVOption options[] = {
+    {"no_resync_search", "Don't try to resynchronize by looking for a certain optional start code", offsetof(ASFContext, no_resync_search), AV_OPT_TYPE_INT, {.dbl = 0}, 0, 1, AV_OPT_FLAG_DECODING_PARAM },
+    { NULL },
+};
+
+static const AVClass asf_class = {
+    .class_name = "asf demuxer",
+    .item_name  = av_default_item_name,
+    .option     = options,
+    .version    = LIBAVUTIL_VERSION_INT,
+};
+
 #undef NDEBUG
 #include <assert.h>
 
@@ -709,7 +725,9 @@  static int ff_asf_get_packet(AVFormatContext *s, AVIOContext *pb)
 
     // if we do not know packet size, allow skipping up to 32 kB
     off= 32768;
-    if (s->packet_size > 0)
+    if (asf->no_resync_search)
+        off = 3;
+    else if (s->packet_size > 0)
         off= (avio_tell(pb) - s->data_offset) % s->packet_size + 3;
 
     c=d=e=-1;
@@ -1293,4 +1311,5 @@  AVInputFormat ff_asf_demuxer = {
     .read_seek      = asf_read_seek,
     .read_timestamp = asf_read_pts,
     .flags          = AVFMT_NOBINSEARCH | AVFMT_NOGENSEARCH,
+    .priv_class     = &asf_class,
 };