[vlc-devel] [PATCH] When the demuxer sets dimensions use them rather than the codec values

[Steve Lhomme]

On 07/03/2015 10:40, Rémi Denis-Courmont wrote:
> Le vendredi 06 mars 2015, 12:25:00 Steve Lhomme a écrit :
>> On Fri, Mar 6, 2015 at 11:38 AM, Rémi Denis-Courmont <remi at remlab.net>
> wrote:
>>> Le 2015-03-06 13:26, Steve Lhomme a écrit :
>>>> On Fri, Mar 6, 2015 at 11:11 AM, Rémi Denis-Courmont <remi at remlab.net>
>>>>
>>>> wrote:
>>>>> Le 2015-03-06 12:25, Steve Lhomme a écrit :
>>>>>> Your first assumption is false. I don't know about MP4 but in Matroska
>>>>>> the coordinate mapping is defined. It applies to the output format of
>>>>>> the codec.
>>>>>
>>>>> The output format of the decoder depends on the decoder implementation.
>>>>> If
>>>>> the Matroska specification depends on that, then the Matroska
>>>>> specification
>>>>> is broken. And my assumption still holds.
>>>>
>>>> No. That has nothing to do with implementation.
>>>
>>> Then the patch is wrong, because of exactly what I was writing earlier:
>>> only the decoder knows the mapping, not the core.
>>
>> As said in the patch, it creates bleeding with some vout. But since
>> you mentioned that maybe the container should overwrite the codec
>> values when they are present, I tried.
>
> I might have argued that the container overriding the crop dimensions from the
> codec would make more sense than substracting them - at the specification. But
> as far as I understand, Matroska specifies the substraction.
>
> I don´t think I said that the VLC demuxer should overwrite the VLC decoder
> values - at the implementation level. My point is that there is only one place
> where you can perform the override or the substraction: the place where you
> have all the necessary informations, including the container format and crop,
> the codec format and crop, and the decoder padding.
>
> That place is within the decoder. From there, container and/or packetizer
> parameters are available in decoder_t.fmt_in.video, the codec crop parameters
> shall be available from the underlying decoding library (namely libavcodec),
> as shall the necessary padding if any. You can combine all that to write out
> the correct decoder.fmt_out.video.

Thanks for the insight. I've been playing around in that area of code 
but would only end up with some green areas or bleeding. But since 
user/manual cropping work in most cases, I went to other way around and 
look at what Direct3D expects and get when it works and when it doesn't 
to find where exactly is the issue. For now it still involves passing 
down some crop data.

> Solving the problem once for all decoder modules in the decoder core would be
> nice, but it is not generally feasible. And there is no need to involve the
> video output for that. So I still claim that you need to "fix" the libavcodec
> plugin, not the core, as I have been claiming for several days.

I would rather have a solution that works in all cases, rather than just 
adding more specific code in libavcodec and pray all the second rate 
citizens deal with it. Even if it takes time, but I'll let j-b decide if 
it worth the effort for the long term.

> As for the DirectX video outputs, or a subset of them, maybe they are buggy; I
> don´t know. But that seems like a separate problem. First you need to get the
> proper video format to the video output, and then you can deal with fixing the
> video output plugins if necessary.

Agreed. Only Direct2D is seriously broken. The others (on Windows) seem 
fine.