[FFmpeg-devel] [PATCH] demux individual program out of MPEG-TS

[Michael Niedermayer]

Hi

On Thu, Sep 13, 2007 at 11:48:22PM +0200, Nico Sabbi wrote:
> Benoit Fouet wrote:
>
>> because of the "continue", i don't know how it will be compiled
>> maybe it will generate the same thing as the code i wrote :)
>
> except this continue stuff I fixed all other points you raised,
> but I want to add that I consider the usefulness of doxygen comments
> very over-estimated: first of all good written comments are equally
> clear and easier to write and read; moreover doxy comments aren't
> really the rule in ffmpeg: just reading 4 random files in livabformat
> I couldn't find any of them properly compliant.

which comments have you found to be non compliant?


[...]
> @@ -926,6 +980,8 @@
>      const uint8_t *p, *p_end;
>  
>      pid = AV_RB16(packet + 1) & 0x1fff;
> +    if(pid && discard_pid(ts, pid))
> +        return;
>      is_start = packet[1] & 0x40;

id slightly prefer if you could split the AVProgram addition from the
AVDiscard addition so they are commited seperately but if you dont want
iam fine with them combined as well


[...]
> -#define LIBAVFORMAT_VERSION_INT ((51<<16)+(12<<8)+2)
> -#define LIBAVFORMAT_VERSION     51.12.2
> +#define LIBAVFORMAT_VERSION_INT ((51<<16)+(12<<8)+3)
> +#define LIBAVFORMAT_VERSION     51.12.3
>  #define LIBAVFORMAT_BUILD       LIBAVFORMAT_VERSION_INT

we need a minor not micro bump as a new stuff is added and sofware which uses
that will not work with old lav*


>  
>  #define LIBAVFORMAT_IDENT       "Lavf" AV_STRINGIFY(LIBAVFORMAT_VERSION)
> @@ -345,6 +345,14 @@
>      int64_t pts_buffer[MAX_REORDER_DELAY+1];
>  } AVStream;
>  
> +typedef struct AVProgram {
> +    int            id;
> +    char           *provider_name; ///< Network name for DVB streams
> +    char           *name;          ///< Service name for DVB streams
> +    int            running;

can this be anything else than 0 and 1 if not a
int flags; #define AV_PROGRAM_RUNNING 1 seems like a better choice


[...]

>  AVStream *av_new_stream(AVFormatContext *s, int id);
> +AVProgram *av_new_program(AVFormatContext *s, int i);

s/i/id/


> +void av_set_program_name(AVProgram *program, char *provider_name, char *name);

do we need that in the public lavf API? (we can always move it to avformat.h
later but we cant remove it easiy once its in there ...)

except these iam fine with the patch, feel free to commit after dealing with
the comments above, and if mans is ok with the changes to mpegts


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

I hate to see young programmers poisoned by the kind of thinking
Ulrich Drepper puts forward since it is simply too narrow -- Roman Shaposhnik
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070923/9d6ee496/attachment.pgp>