On 2008-07-19, David Flynn <[EMAIL PROTECTED]> wrote:
> diff --git a/schroedinger/schroencoder.c b/schroedinger/schroencoder.c
> index 8c64a5c..5f523f7 100644
> --- a/schroedinger/schroencoder.c
> +++ b/schroedinger/schroencoder.c
> @@ -484,6 +484,10 @@ schro_encoder_pull_full (SchroEncoder *encoder, int
> *presentation_frame,
> + if (priv) {
> + *priv = NULL;
> + }
> +
First of all, when a user supplies a priv pointer and we don't have
anything to put back, ensure it gets set to NULL. (this stops hidden
end user bugs).
> @@ -493,7 +497,7 @@ schro_encoder_pull_full (SchroEncoder *encoder, int
> *presentation_frame,
> if (presentation_frame) {
> - *presentation_frame = frame->presentation_frame;
> + *presentation_frame = frame->frame_number;
I'd love to know what on earth presentation_frame is. From my vague
searchings it isn't very well related to anything (it is a munged form
of frame_number, and has little to do with presentation). I can't see
how it is of any use to the user.
> diff --git a/gst/gstschroenc.c b/gst/gstschroenc.c
gstschroenc previously did something along these lines:
- when the stream starts, work out the initial timestamp
and save it as an offset for later
- for each output thing, generate a timestamp by adding
the offset to a number generated by taking the number
of pictures coded so far (picture_number) and multiplying
by framerate[1].
- everytime a seqhdr comes along (and thereby a syncpoint),
munge part of the granulepos.
[1] - i'll ignore the interlaced implications of that for now.
To me, this seems somewhat strage, especially since there are the schro
functions:
- ...push_frame_full()
- ...pull_full()
Both of which allow a user to supply a void* value when a picture enters
the encoder and have it returned when it leaves as a buffer. By doing
this we can loose anything to do with offsets - we never lose time
values now, so don't have to reconstruct them.
> + /* all of this tat is for ogg */
> + uint32_t num_coded_pics; /* number of codec pictures output */
> + uint32_t last_sync_dpn; /* last picnum (in display order) that is a sync
> point */
> + uint32_t last_sync_cpn; /* value of num_coded_pictures at last sync point
> */
If it weren't for ogg[2] we woudln't need anything that referenced these
values.
[2] -- http://article.gmane.org/gmane.comp.video.gstreamer.devel/21275
I've tested this with:
- oggmux: it works fine :)
- tsmux: it required a mod address a FIXME (allow PTS to jump around a bit)
An additional comment should be noted with tsmux: as far as i can tell,
it isn't possible to correctly tsmux a stream with gstreamer. The MPEG
rules require if PTS != DTS, then both be signalled. However, a buffer
only has one timestamp -- the [effective] PTS, i haven't been able to
find something that can express DTS.
NB, given a sequence of PTS values it isn't trivial [possibly not
possible (certainally not a good idea)] to solve for DTS.
thoughts, comments, etc.,
..david
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
Schrodinger-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/schrodinger-devel
