<div dir="ltr">2013/10/23 Martin Storsjö <span dir="ltr"><<a href="mailto:martin@martin.st" target="_blank">martin@martin.st</a>></span><br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">

<div class="im">On Wed, 23 Oct 2013, Felix Abecassis wrote:<br>
<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Using VLC type for lookup in chroma_format_table is not enough since<br>
there can be several corresponding OMX types with different<br>
dimensions.<br>
---<br>
modules/codec/omxil/android_<u></u>mediacodec.c | 2 +-<br>
modules/codec/omxil/omxil.c              | 8 ++++----<br>
modules/codec/omxil/omxil_<u></u>utils.h        | 2 +-<br>
modules/codec/omxil/utils.c              | 8 +++-----<br>
4 files changed, 9 insertions(+), 11 deletions(-)<br>
</blockquote>
<br></div>
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).<br></blockquote><div>Ah, yes, a trailing whitespace that was deleted by my config.<br>

</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
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?<br></blockquote><div>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(...).<br>

</div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
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.<br>

</blockquote><div>Can someone double check the entries in the chroma_format_table?<br></div><div>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.<br>

</div><div>If the dimensions only depends on the VLC type then we could have a separate table mapping a VLC type to dimensions information.<br><br></div><div>Thanks for your feedback,<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">


<br>
So if the table is fixed, I'm not sure that the different formats actually can/should have different dimensions.<br>
<br>
Despite that, the patch might still be useful, but first we need to know exactly what you are trying to fix here.<br>
<br>
// Martin<br>
______________________________<u></u>_________________<br>
vlc-devel mailing list<br>
To unsubscribe or modify your subscription options:<br>
<a href="https://mailman.videolan.org/listinfo/vlc-devel" target="_blank">https://mailman.videolan.org/<u></u>listinfo/vlc-devel</a><br>
</blockquote></div><br><br clear="all"><br>-- <br>Félix Abecassis<div><a href="http://felix.abecassis.me" target="_blank">http://felix.abecassis.me</a></div>
</div></div>