[Bug 796818] deinterlace: Timecode pass-through

[GStreamer (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=796818

--- Comment #7 from Sebastian Dröge (slomo) <slomo at coaxion.net> ---
(In reply to Sebastian Dröge (slomo) from comment #6)
> Review of attachment 373066 [details] [review]:
> 
> ::: gst/deinterlace/gstdeinterlace.c
> @@ +1127,3 @@
> +    self->field_history[i].tc = self->field_history[i - fields_to_push].tc;
> +    self->field_history[i].has_tc =
> +        self->field_history[i - fields_to_push].has_tc;
> 
> The old timecodes have to be cleared here
> 
> @@ +1171,3 @@
> +
> +    if (meta) {
> +      self->field_history[0].tc = meta->tc;
> 
> And here need to take ownership of the data inside the timecode, or it might
> go away together with the buffer

I actually noticed that this is fine. The buffer is kept alive all the time
until you stop using the timecode. Please add a comment but otherwise keep it
:)

> @@ +1165,3 @@
>      GST_DEBUG_OBJECT (self, "Two fields");
>      self->field_history[1].frame = field1;
>      self->field_history[1].flags = field1_flags;
> 
> You probably also want to add the timecode here

This is still valid

> @@ +1768,3 @@
> +          &self->field_history[index].tc);
> +    } else {
> +      GstVideoTimeCodeMeta *meta = gst_buffer_get_video_time_code_meta
> (buf);
> 
> This part is unneeded if you put the timecode into index 0 and index 1 above
> where things are put into the field history

And this

> @@ +1777,3 @@
> +      if (meta) {
> +        meta->tc.config.fps_n = 2 * meta->tc.config.fps_n;
> +        meta->tc.frames = 2 * meta->tc.frames;
> 
> These calculations are probably wrong for telecine but I don't know how they
> should be. They are probably also wrong if fields like ONEFIELD and RFF are
> in the stream (duplicated timecodes and timecode gaps will happen then).
> 
> And I believe also without the field=all mode, ONEFIELD/RFF could cause
> non-contiguous timecodes here

For these cases, we discussed and couldn't come up with a reasonable way of
doing timecodes in those situations. What happens here right now is probably as
good as any other alternative. Should probably just get a GST_FIXME just in
case... but only once, not per frame ;)

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.