[gst-devel] MPEG demux restructuring

[Ronald Bultje]

Hi Martin,

On Tue, 23 Mar 2004, Martin Soto wrote:
> - Restrict mpegdemux to pure MPEG. DVD specific extensions are not
> handled directly, but just written to the private_stream source pads.
> Audio pads are exclusively for MPEG audio, so you don't have any special
> pads for other audio types.

Looks good, mostly. As for the timing, I noticed you've noticed the same
oddness (scr adjustment from stream) in mpegdemux that I noticed some time
ago. I;'ve put a patch in a low-number (#99...) bug in bugzilla, which
removes it completely. Please use that instead. A property is not a good
idea here, the whole scr adjustment thing is broken, imo.

Apart from that, it needed this cleanup. DVD stuff belongs somewhere else.

> - Add a new dvddemux element, that actually handles DVD extensions. This
> element provides dynamic video, audio and subpicture source pads, that
> are created as new substreams are found in the main MPEG stream. The
> same audio pads are used for all audio types, and negotiate caps as
> necessary when/if audio type changes on the fly.

Some comments:
- cur_video etc. seem to be actual realpads, not ghostpads. That's *not*
good, imo. Make them ghostpads to their respective audio/video/priv pads
and use those instead. Also, I'd love it if you would rename the pads
in dvddemux to audio_%02d etc. Use GST_MPEG_DEMUX (dvddemux)->numaudiopads
to create them. They *are* audio pads. Pad names should indicate types,
not the caps that will be set on it. ;).
- in dvddemux, in the discont event function, you seem to actually do
specific handling for the current video/audio pad. I'm not sure about
this, but my personal opinion is that you shouldn't do that. Always handle
all pads, that's GStreamer. I want to be able to use all audio streams and
all whatever streams if I have to. That's why current_pad should be a
ghost pad, not a real pad. I skimmed quickly and couldn't find other
places where you do this, but if there is any: please don't. I want all
streams available. MPEG is specifically made for this type of stream
handling (multi-video, multi-audio, ..).
- Style issue: I don't like your CLASS(), MPEG_CLASS() etc. macros. Please
just use the full names (GST_MPEG_PARSE_GET_CLASS(), which can be called
on a GstDvdDemux without casting, too).

> - Additionally, the dvddemux element has a notion of current video,
> audio and subpicture streams, that can be selected trough events (I'll
> also add properties for that). current_video, current_audio, and
> current_subpicture source pads give access to the current streams,
> which, of course, are also available through the standard dynamic pads.

How do you want to do that? :). Properties? Interface? ...? Integration in
pads (I'm in favour of this one! matroska, normal mpeg et all have the
exact same issue...).

Ronald