[FFmpeg-devel] [PATCH] Private options for url protocols

[Michael Niedermayer]

On Thu, Nov 10, 2011 at 06:07:06PM +0100, Clément Bœsch wrote:
> On Thu, Nov 10, 2011 at 05:56:11PM +0100, Michael Niedermayer wrote:
> > Hi
> > 
> > Attached patchset implemets subj
> > Comments welcome, ill probably push it tomorrow if there are no
> > objections
> > 
> > An example of how useage would look is:
> > ffplay http,user-agent=JustTesting://samples.multimedia.cx/MPEG2/test422.m2v
> > 
> > the syntax would also work for nested protocols ...
> > 
> 
> There was a pending work posted on libav-devel today for this
> (http://lists.libav.org/pipermail/libav-devel/2011-November/014324.html),

hmm, i hadnt seen that yet


> could you comment on the differences if it indeed has the same purpose?

well, sure, mine needs 10 lines in one file, thats all:
        avio.c |   16 +++++++++++++---
        1 file changed, 13 insertions(+), 3 deletions(-)

anton khirnovs:
    libavformat/http.c |   18 +++++++++++-------
    1 files changed, 11 insertions(+), 7 deletions(-)
    libavformat/avio.c |   19 +++++++++++--------
    libavformat/url.h  |    7 +++++++
    2 files changed, 18 insertions(+), 8 deletions(-)
    libavformat/options.c |   17 ++++++++++++++---
    libavformat/utils.c   |    6 +++---
    2 files changed, 17 insertions(+), 6 deletions(-)
    libavformat/applehttp.c      |    6 ++--
    libavformat/applehttpproto.c |    2 +-
    libavformat/avio.c           |   55 +++++++++++++++++++++++++++++++++++-------
    libavformat/aviobuf.c        |    2 +-
    libavformat/concat.c         |    3 +-
    libavformat/crypto.c         |    2 +-
    libavformat/gopher.c         |    3 +-
    libavformat/http.c           |    3 +-
    libavformat/md5proto.c       |    3 +-
    libavformat/mmsh.c           |    4 +-
    libavformat/mmst.c           |    2 +-
    libavformat/rtmpproto.c      |    2 +-
    libavformat/rtpproto.c       |    4 +-
    libavformat/rtsp.c           |   14 +++++-----
    libavformat/sapdec.c         |    3 +-
    libavformat/sapenc.c         |    5 ++-
    libavformat/tls.c            |    3 +-
    libavformat/url.h            |   20 +++++++++++++-
    18 files changed, 98 insertions(+), 38 deletions(-)

--------------
And antons needs a major ABI bump:
   "2) Passing options from AVFormatContext or AVIOContext to protocols by
    me. This will start working after the next major bump, because it needs
    an AVClass to be added to the beginning of AVIOContext..."

Another difference is that antons private options all require
duplicated AVClasses, if you reuse one the code will end in an infinite
loop. This makes patches like
 [PATCH 03/18] http: use different classes for http and https.
needed

His implementations for (de)muxers and codecs have the same
problems.
The infinite looping happens because the enummeration function he
has written works by searching through the whole list of codec/muxers
and matches their classes against the current entry and then returns
the next. (see codec_child_class_next() for example)
That way enumeration is O(n^2) when there are no duplicate classes
and O(infinite) if there are duplicates.
I suggested him to use a next pointer when i saw the issue happen in
i belive in a decoder, but i never received a reply.
If one looks at the whole command line parsing, then parsing n
command line arguments, and assuming there are n AVClasses with 1
options each. Command line parsing takes at least O(n^3)
you might have noticed that the naive implementation just needs O(n^2)
and it could of course be done in O(n)
Does it matter in reality? maybe not yet. But doing a billion
operations when a thousand is enough feels wrong to me.

Above is not intended to be a flame at all, i just would like to see
this mess fixed before it spreads further.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Rewriting code that is poorly written but fully understood is good.
Rewriting code that one doesnt understand is a sign that one is less smart
then the original author, trying to rewrite it will not make it better.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20111110/01131cfa/attachment.asc>