[vlc-devel] [PATCH] mmal: update the code from the one actually used in Raspbian

[Steve Lhomme]

Hi,

On 2020-01-15 18:27, Rémi Denis-Courmont wrote:
> Le keskiviikkona 15. tammikuuta 2020, 17.46.31 EET Steve Lhomme a écrit :
>> From: John Cox <jc at kynesim.co.uk>
>>
>> Tested with basic video playback on a RPI 3B+
>>
>> This is the code from the Raspbian patch ported to the 4.0 architecture.
>> It includes an OpenGL interop module and some NEON optimizations.
> 
> I don't think it's appropriate to make a big code dump. That's too hard to
> review properly.

I could split the code in smaller patches. In the end it will be exactly 
the same amount of code to review, if not more given patches include 
unchanged lines and changes might affect the same lines multiple times.

I already removed a big chunk of the original sources to just pick the 
parts we need to have something working.

> And it's questionable why to merge the code at all. If the author could not be

We have non working MMAL support in master. So either we drop support 
completely or we fix it.

If we decide to keep supporting MMAL (I'm OK to do it) then we should 
(re)start from the code that people actually use in Raspbian, the 
default RPI distribution. This code was also sponsored by Raspberry Pi.

That code is made to work on 3.0.8 but I imported it in 4.0 and modified 
it so it compiles and use the proper new APIs. I started working on a 
decoder device and video context but it's not ready yet.

The old MMAL code we have makes use of the display pool which won't work 
anymore, so we have to do something about it. That's why I wanted to 
clean this code so we can finally remove the last vd->pool in the code.

> bothered to submit it themselves, then there's no reason to believe that it

It's designed for 3.0. Porting decoder/vout code to 4.0 is not an easy 
thing.

> won't immediately bit-rot, and the same problems won't occur again and again.

If we decide to support it I'd like to add a Raspbian target in the 
buildbot so we don't forget to update/break it in the future.

The current status is that we have code, that barely compiles and 
doesn't work. This code is also not used by anyone. Replacing it with 
something else won't make it worse.

>> diff --git a/src/misc/fourcc.c b/src/misc/fourcc.c
>> index 9b4ea14c31b..ef83ac3b4ad 100644
>> --- a/src/misc/fourcc.c
>> +++ b/src/misc/fourcc.c
>> @@ -816,8 +816,13 @@ static const struct
>>       { { VLC_CODEC_VDPAU_VIDEO_420, VLC_CODEC_VDPAU_VIDEO_422,
>>           VLC_CODEC_VDPAU_VIDEO_444, VLC_CODEC_VDPAU_OUTPUT },
>>                                                  FAKE_FMT() },
>> -    { { VLC_CODEC_ANDROID_OPAQUE, VLC_CODEC_MMAL_OPAQUE,
>> -        VLC_CODEC_D3D9_OPAQUE,    VLC_CODEC_D3D11_OPAQUE },
>> +    { { VLC_CODEC_ANDROID_OPAQUE },            FAKE_FMT() },
>> +    { { VLC_CODEC_MMAL_OPAQUE, VLC_CODEC_MMAL_ZC_SAND30   },
>> +                                               FAKE_FMT() },
>> +    { { VLC_CODEC_MMAL_ZC_I420,   VLC_CODEC_MMAL_ZC_SAND8,
>> +        VLC_CODEC_MMAL_ZC_SAND10, VLC_CODEC_MMAL_ZC_RGB32 },
>> +                                               FAKE_FMT() },
>> +    { { VLC_CODEC_D3D9_OPAQUE,    VLC_CODEC_D3D11_OPAQUE },
> 
> That's a lot of entries for a single piece of hardware. We don't normally
> expose specific hardware pixel formats.
> 
>>                                                  FAKE_FMT() },
>>       { { VLC_CODEC_D3D11_OPAQUE_10B, VLC_CODEC_D3D9_OPAQUE_10B,
>>           VLC_CODEC_D3D11_OPAQUE_RGBA, VLC_CODEC_D3D11_OPAQUE_BGRA },
> 
> 
> -- 
> Rémi Denis-Courmont
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
>