Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DVDVideoCodecDRMPRIME: update process info in get format callback #15964

Merged
merged 2 commits into from
Apr 29, 2019

Conversation

Kwiboo
Copy link
Member

@Kwiboo Kwiboo commented Apr 22, 2019

Description

This PR adds support for using threads when decoding, changes to update process info in get format callback and fixes a potential issue in case v4l2 request api hwaccel fails to init and sw pix fmt is used.

Motivation and Context

Fixes wrong info in process info dialog (pix fmt, hw/sw) in some cases and prepares for sw decode fallback in case hwaccel fails to init.

How Has This Been Tested?

Tested on Allwinner and Rockchip devices using LibreELEC and mainline kernel.

Screenshots (if appropriate):

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Clean up (non-breaking change which removes non-working, unmaintained functionality)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that will cause existing functionality to change)
  • Cosmetic change (non-breaking change that doesn't touch code)
  • None of the above (please explain below)

Checklist:

  • My code follows the Code Guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the Contributing document
  • I have added tests to cover my change
  • All new and existing tests passed

@Kwiboo
Copy link
Member Author

Kwiboo commented Apr 23, 2019

Dropped the threaded decoding commit, it caused issues with 4K on Allwinner devices, something to improve in future.

@lrusak lrusak self-assigned this Apr 29, 2019
@lrusak lrusak added Component: Video Platform: Linux Type: Improvement non-breaking change which improves existing functionality v19 Matrix labels Apr 29, 2019
@lrusak lrusak added this to the M** 19.0-alpha 1 milestone Apr 29, 2019
@lrusak
Copy link
Contributor

lrusak commented Apr 29, 2019

@Kwiboo was the issue with threaded decoding with a hw decoder or sw decoder?

@Kwiboo
Copy link
Member Author

Kwiboo commented Apr 29, 2019

I think the issue was memory constraints for hw decoder, with 4k, multiple threads and each thread allocating extra buffers it was too much on the board @jernejsk tested.
Means we probably need to have configuration or algo to determine thread usage.

@jernejsk
Copy link
Contributor

@lrusak issue is that too much CMA memory is requested when decoding 4K videos. It seems that ffmpeg makes multiple instances of v4l2_request codec.

@lrusak
Copy link
Contributor

lrusak commented Apr 29, 2019

I think the issue was memory constraints for hw decoder, with 4k, multiple threads and each thread allocating extra buffers it was too much on the board @jernejsk tested.
Means we probably need to have configuration or algo to determine thread usage.

Ah, makes sense. Are we going to be able to set the threading if we are using SW decoding?

@lrusak issue is that too much CMA memory is requested when decoding 4K videos. It seems that ffmpeg makes multiple instances of v4l2_request codec.

Are you able to decrease the buffer pool size? How much CMA are you currently allocating?

@jernejsk
Copy link
Contributor

jernejsk commented Apr 29, 2019

Are you able to decrease the buffer pool size? How much CMA are you currently allocating?

ffmpeg already allocates only on-demand, but @Kwiboo might correct me on that one. Currently I set 256 MiB for CMA, but I don't want to increase that. Older AW SoCs are not even capable of using more or even that amount of memory for VPU.

@lrusak
Copy link
Contributor

lrusak commented Apr 29, 2019

Are you able to decrease the buffer pool size? How much CMA are you currently allocating?

ffmpeg already allocates only on-demand, but @Kwiboo might correct me on that one. Currently I set 256 MiB for CMA, but I don't want to increase that. Older AW SoCs are not even capable of using more or even that amount of memory for VPU.

256MiB should be plenty no? (256 * 1,000,000) / (3840 * 2160) = ~30 or am I out to lunch?

@jernejsk
Copy link
Contributor

You forgot to divide by 1.5 - NV12 has 12 bpp, but that's still 20, which should be enough for all videos. Worst case would be 16 images in DPB and 1 decoding. That still leaves 3 for buffering purposes, if needed. However, I haven't seen video yet which would use 16 reference frames...

@Kwiboo
Copy link
Member Author

Kwiboo commented Apr 29, 2019

V4l2 request api hwaccel also allocates 2 mb bitstream buffer for each frame buffer so it adds up, though it was easier to leave threading to a dedicated PR in near future

@lrusak
Copy link
Contributor

lrusak commented Apr 29, 2019

You forgot to divide by 1.5 - NV12 has 12 bpp, but that's still 20, which should be enough for all videos. Worst case would be 16 images in DPB and 1 decoding. That still leaves 3 for buffering purposes, if needed. However, I haven't seen video yet which would use 16 reference frames...

Right 😸

V4l2 request api hwaccel also allocates 2 mb bitstream buffer for each frame buffer so it adds up, though it was easier to leave threading to a dedicated PR in near future

No problem, just thought I would ask 👍

@jernejsk
Copy link
Contributor

ah, true and we must not forget other users of CMA. For example, fbcon also allocates one frame to show console. Some other drivers use it too, but I'm not sure how much.

@lrusak lrusak merged commit d75132d into xbmc:master Apr 29, 2019
@Kwiboo Kwiboo deleted the drmprime-process-info branch May 1, 2019 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Video Platform: Linux Type: Improvement non-breaking change which improves existing functionality v19 Matrix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants