rust-av / ffms2-rs

ffms2 bindings
MIT License
10 stars 5 forks source link

Frame::get_pixel_data is using wrong slice length? #28

Closed ben476 closed 1 year ago

ben476 commented 1 year ago

I've noticed that the calculation for plane_slice_length on this line doesn't seem to account for vertical subsampling (not sure what the actual term is, I'm new to this), causing the U and V plane slice lengths to be double of what they should be for yuv420p videos. linesize is correctly accounting for horizontal subsampling though.

As a workaround, on yuv420p videos I've been setting linesize for the U and V planes to half of what they should be:

frame.Linesize[1] /= 2;
frame.Linesize[2] /= 2;
Luni-4 commented 1 year ago

Thanks for spotting that! Unfortunately I can't work on that in the next period but I'm accepting PRs

ben476 commented 1 year ago

I can't seem to find a way to get the vertical chroma subsampling from the ffms2 API, although I can see that it uses AVPixFmtDescriptor internally. Do you know how it's supposed to be accessed?

Luni-4 commented 1 year ago

You can access to those data through the FFMS_Frame properties

ben476 commented 1 year ago

I see there's int ConvertedPixelFormat, but it's just the enum without the AVPixFmtDescriptor struct, so I'm not too sure how to get the vertical chroma subsampling from it.

ben476 commented 1 year ago

I don't think including some sort of map in the file would be practical since there are 222 different pixel formats in the AVPixelFormat enum. Parsing the pixel format name might also be a challenge since there isn't an opposite of FFMS_GetPixFmt.

Luni-4 commented 1 year ago

Just wondering about this function though, the exact width and height should already be computed considering scaled and non-scaled context, so dividing by two the linesize shouldn't be necessary. Could you please double-check with other videos and formats too? Just to verify, thanks!

ben476 commented 1 year ago

I think it's correct for the Y plane, but the function doesn't change for U and V planes which will be vertically subsampled and have a different resolution for pixel formats such as yuv420p.

I've just checked with some more yuv420p videos and am still having the issue. Videos that use yuv422p instead (no vertical chroma subsampling) are fine though.

Luni-4 commented 1 year ago

Looking at this test we can see that ConvertedPixelFormat has been used to get AVPixFmtDescriptor and then those two lines to obtain the chroma subsampling for width and height dimensions.

So in order to retrieve those information it's necessary to use FFmpeg, you can try https://github.com/shssoichiro/ffmpeg-the-third which are the new bindings for FFmpeg library.