1146221 - [Video][dolphin][FFOS7715 v2.1] fail to parse thumbnail. (caused by bug#1141936)

Status: RESOLVED FIXED

(Core :: Audio/Video, defect)

Description

[Wenyuan Chi [:wchi]]

Attachment: fail_to_parse_thumbnail_show_only_defalut_icon.png

After applying patch of bug#1141936, video app fails to parse thumbnail.

Comment 1

[Wenyuan Chi [:wchi]]

Attachment: fix fail to parse video thumbnail.

Comment 2

[Wenyuan Chi [:wchi]]

Attachment: hg.patch

change OmxMapping of HAL_PIXEL_FORMAT_YCbCr_420_SP to zero to use ConvertVendorYUVFormatToRGB565()

Comment 3

[Vincent Liu[:vliu]]

Comment on attachment 8581417 [details] [diff] [review]
fix fail to parse video thumbnail.

Review of attachment 8581417 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

Comment 4

[Vincent Liu[:vliu]]

This patch intends for bug#1141936. 2.1s?

Comment 5

[Vincent Liu[:vliu]]

https://hg.mozilla.org/releases/mozilla-b2g34_v2_1s/rev/641335274d31

Comment 6

[[e-mail]]

(In reply to wchi from comment #2)
> Created attachment 8581420 [details] [diff] [review]
> hg.patch
> 
> change OmxMapping of HAL_PIXEL_FORMAT_YCbCr_420_SP to zero to use
> ConvertVendorYUVFormatToRGB565()
Dear wenyuan:

After applying the new patch, the thumbnail display default issue and green stripe issue both fixed perfectly, really thanks for your help.

But I just want to know why are we should change OmxMapping of HAL_PIXEL_FORMAT_YCbCr_420_SP to zero to use ConvertVendorYUVFormatToRGB565(), What principle is about this? I want to learn from it, could you have some further detail discription?

Thank you so much!

Comment 7

[Wenyuan Chi [:wchi]]

(In reply to [e-mail] from comment #6)
> (In reply to wchi from comment #2)
> > Created attachment 8581420 [details] [diff] [review]
> > hg.patch
> > 
> > change OmxMapping of HAL_PIXEL_FORMAT_YCbCr_420_SP to zero to use
> > ConvertVendorYUVFormatToRGB565()
> Dear wenyuan:
> 
> After applying the new patch, the thumbnail display default issue and green
> stripe issue both fixed perfectly, really thanks for your help.
> 
> But I just want to know why are we should change OmxMapping of
> HAL_PIXEL_FORMAT_YCbCr_420_SP to zero to use
> ConvertVendorYUVFormatToRGB565(), What principle is about this? I want to
> learn from it, could you have some further detail discription?
> 
> Thank you so much!

Hi Lin,

It is reported that there is a green bar in the image. It indicates that there could be something wrong about Y channel and UV channel position.

After digging, I found that the pointer of UV channel queried by GraphicBuffer::lockYCbCr() [1] is different than the pointer computed by libstagefright [2] 

Because of that, I think it is more reasonable to treat HAL_PIXEL_FORMAT_YCbCr_420_SP as a vendor specific format instead of OMX format. That is why I change the mapping to zero.

By doing so, we can use ConvertVendorYUVFormatToRGB565() to handle the vendor specific format. [3]

[1] http://mxr.mozilla.org/mozilla-b2g34_v2_1/source/gfx/layers/GrallocImages.cpp#228
[2] https://github.com/mozilla-b2g/platform_frameworks_av/blob/v2.1s/media/libstagefright/colorconversion/ColorConverter.cpp#L382
[3] http://mxr.mozilla.org/mozilla-b2g34_v2_1/source/gfx/layers/GrallocImages.cpp#404

Comment 8

[[e-mail]]

(In reply to wchi from comment #7)
> It is reported that there is a green bar in the image. It indicates that
> there could be something wrong about Y channel and UV channel position.
> 
> After digging, I found that the pointer of UV channel queried by
> GraphicBuffer::lockYCbCr() [1] is different than the pointer computed by
> libstagefright [2] 
> 
> Because of that, I think it is more reasonable to treat
> HAL_PIXEL_FORMAT_YCbCr_420_SP as a vendor specific format instead of OMX
> format. That is why I change the mapping to zero.
> 
> By doing so, we can use ConvertVendorYUVFormatToRGB565() to handle the
> vendor specific format. [3]
Thanks for your reply about my question, next I will learn this code related.