[vlc-devel] [PATCH] Fix deinterlacing in transcode pipeline.

[Rémi Denis-Courmont]

Le mardi 11 juin 2013 21:02:19, Steinar H. Gunderson a écrit :
> On Tue, Jun 11, 2013 at 08:38:13PM +0300, Rémi Denis-Courmont wrote:
> >> 0002-In-transcoding-move-frame-output-into-its-own-functi.patch
> > 
> > s/nad/and/
> 
> Done.
> 
> >> 0003-Make-the-video-transcoder-support-filter-chains-that.patch
> > 
> > It would be a lot easier if there were a single chain. Oh well...
> 
> Yes, I haven't really understood why they are not. I've heard about static
> vs. dynamic somehow, but I can't find anything that actually changes any of
> them on-the-fly.

No. It ended up that way while following the path of least resistance. Now 
that the corner case of non 1:1 filters is taken into account, this is no 
longer justified.

Besides if the user filters chain became dynamic, the non-user filters chain 
would also need to be adjusted dynamically.

> >> 0004-In-video-transcode-use-the-correct-format-for-calcul.patch
> > 
> > Why does the frame rate get special treatment here? The comments insists
> > on it but fails to give a rationale.
> 
> I guess the question here is: Do we want this to be the filter output
> format with width/height/chroma/SAR/etc. changed, or the encoder input
> format with the frame rate changed? I would guess they are mostly
> equivalent, but there are a lot of changes done in
> transcode_video_filter_init() that we'd need to pick up.

If you create the filters before the encoder, this question should be moot.
The encoder just gets the filtered format, including the filtered frame rate.

***

> >> 0005-Set-up-the-encoder-after-the-transcode-filter-chain.patch
> > 
> > That seems logical. However traditionally, VLC has initialized the
> > "output" of the chain against the input format. Indeed, the same method
> > is applied for regular video and audio filter chains.
> > 
> > I don't mind when it comes to transcode. But I gave up "fixing" this for
> > audio; at the very least, it would have broken S/PDIF pass-through.
> 
> I'm not entirely sure what you are referring to here; by “output”, do you
> mean the filter chain or the encoder?

I mean the object that gets the result of the filters chain, here in the 
encoder.

> Generally I'm not sure what I'm doing
> that would be in conflict with what you say.
> 
> As an aside, I'm a bit surprised at the amount of code duplication between
> the transcode and vout chains; in retrospect it's less surprising that
> yadif2x was broken in one but not the other.

The filter chain code was supposed to solve this. But the unification was never 
finished, and later the video output core was rewritten from scratch.

This should probably be unified like I did the audio filters recently. And 
afterward, audio, video and sub filters should be partly unified. But that 
sounds more like a world domination scheme than reality.

> >> 0006-Fix-deinterlacing-of-packed-YUV-formats.patch
> > 
> > Well, it works but ideally it would not be hard-coded in transcode. The
> > same problem exists in the regular video output, but worse. I claim it
> > is worse as the video output chroma cannot be forced like the stream
> > output chroma (transcode{vcodec=I420}).
> 
> Well, ideally filter_chain_AppendFilter() would take care of this...

Ideally, the deinterlacer would support common packed formats. Algorithms that 
do not straddle across columns should easily deal with packed formats.

Besides, more generally, video filters (not just deinterlace) should request an 
input format change when unavoidable. Audio filters already do exactly that. 
Then this kludge^Wspecial case would be unnecessary.

> I guess I could make a utility function in deinterlace.c that did basically
> AppendDeinterlaceFilter but used inside knowledge of the allowed formats
> (the top part of Open()) to avoid resetting the chain all the time?

For audio this was solved by separating conversion and filtering. Then filters 
are allowed to change both input and output formats, and converters are not 
allowed to change any of them both. The filter chain code knows how to split 
non-trivial format conversion into multiple steps, or discard a filter if all 
else fails.

> I have
> no idea how this works if the deinterlace module for some reason is not
> available, though (I don't know the VLC module system very well).

Complete rewrite of video filter is not on the menu at this time. However 
adding YUY2 to the deinterlacer cannot be so hard.

-- 
Rémi Denis-Courmont
http://www.remlab.net/