[vlc-devel] [PATCH] omxil: use OMX chroma type instead of VLC type in GetVlcChromaSizes.

[Felix Abecassis]

2013/10/23 Martin Storsjö <martin at martin.st>

> On Wed, 23 Oct 2013, Felix Abecassis wrote:
>
>  Using VLC type for lookup in chroma_format_table is not enough since
>> there can be several corresponding OMX types with different
>> dimensions.
>> ---
>> modules/codec/omxil/android_**mediacodec.c | 2 +-
>> modules/codec/omxil/omxil.c              | 8 ++++----
>> modules/codec/omxil/omxil_**utils.h        | 2 +-
>> modules/codec/omxil/utils.c              | 8 +++-----
>> 4 files changed, 9 insertions(+), 11 deletions(-)
>>
>
> The patch itself doesn't look bad (but you've got a stray change at the
> end of utils.c, but that can be fixed by whoever applies it, if it should
> be applied).
>
Ah, yes, a trailing whitespace that was deleted by my config.


> However, what I'm unsure of is which case this actually fixes. Is there
> any current config that is broken that is fixed by this patch?
>
No, but it looks like the problem can happen for a generic OMXIL module
(omxil.c). For instance with a
GetVlcChromaFormat(OMX_TI_COLOR_FormatYUV420PackedSemiPlanar, ...) followed
by a GetVlcChromaSizes(...).


> The only place in chroma_format_table where one VLC chroma has got
> differing values depending on the OMX chroma format seems to be the values
> for VLC_CODEC_NV12, and the value for
> OMX_TI_COLOR_FormatYUV420PackedSemiPlanar might actually be wrong, since
> they should all have the same value for i_line_chroma_div.
>
Can someone double check the entries in the chroma_format_table?
Is it also true for i_size_mul and i_line_mul? If this is the case then it
seems confusing and error-prone to have one table with VLC type, OMX type
and dimensions.
If the dimensions only depends on the VLC type then we could have a
separate table mapping a VLC type to dimensions information.

Thanks for your feedback,


>
> So if the table is fixed, I'm not sure that the different formats actually
> can/should have different dimensions.
>
> Despite that, the patch might still be useful, but first we need to know
> exactly what you are trying to fix here.
>
> // Martin
> ______________________________**_________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/**listinfo/vlc-devel<https://mailman.videolan.org/listinfo/vlc-devel>
>



-- 
Félix Abecassis
http://felix.abecassis.me
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20131023/222150f4/attachment.html>