New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 721674 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Bug

Blocked on: View detail
issue 909561
issue 916807



Sign in to add a comment

Support MemoryInfra for HW video codec allocations on CrOS

Project Member Reported by posciak@chromium.org, May 12 2017

Issue description

We should see if we could track HW video codec memory usage from memory-infra (https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/README.md).
 
Owner: hiroh@chromium.org
Status: Assigned (was: Untriaged)

Comment 2 by hiroh@chromium.org, Jun 5 2017

Status: Started (was: Assigned)

Comment 3 by hiroh@chromium.org, Jun 5 2017

I start to add functions to truck driver allocated memory first.
The code review page is crrev.com/2909533003
I just implemented the function to truck driver allocated MMAP memory used in V4L2 VEA/VDA.
No test is done yet.
The next step is:
1) Writing test for V4L2 MMAP memory
2) Support vaSurface used in VA VEA/VDA.

Comment 4 by hiroh@chromium.org, Jun 12 2017

It seems MMAP memory in V4L2 VDA/SVDA/VEA are dumped correctly, seeing chrome://tracing (memory-infra).
I am going to implement the class to dump vaSurface this week.

Comment 5 by hiroh@chromium.org, Jun 23 2017

I fixed bug in V4L2MMAP memory. It looks like works completely correctly. The class is under review, mainly discussing its design.
The first patch about memory class for vaSurface was uploaded on 16th June.
Although some bugs are fixed, the patch still has a bug.
The below would be a hint surely. I am going to fix this bug next week.
[1:11:0623/182634.138020:ERROR:render_media_log.cc(30)] MediaEvent: MEDIA_ERROR_LOG_ENTRY {"error":"video decode error"}
[1:1:0623/182634.483476:ERROR:render_media_log.cc(30)] MediaEvent: PIPELINE_ERROR PIPELINE_ERROR_DECODE

Comment 6 by hiroh@chromium.org, Jun 27 2017

The abovementioned bug was finally fixed.
It turns out the bug was caused by operating to deleted key node in unordered_map (i.e. unordered_map.end()).
This error can happen even in the first patch. I also updated the first patch.

It only tested decoder for now.
I will test encoder and mix of decoder and encoder this week.

Comment 7 by hiroh@chromium.org, Jun 30 2017

Yesterday and the day before yesterday, I redesign VaapiWrapper class so that it does not have a member variable to store va_context_id.
Instead, VASurface has the context id which is associated with the VASurface.

Regarding the bug in encoding, I haven't succeeded in debugging yet.
However, I noticed a bug exists in jpeg decoding, not video encoding.
I tested encoding in appr.tc.
Because debugging was difficult only from logs, I decided to use unittest to find which mechanism has a bug, jpeg decoding and video encoding (perhaps both).
As a result, the jpeg decoding unittest was failed, while video encoding unittest was successful.
I am going to debug focusing jpeg decoding next week.

I haven't executed unittest in the V4L2MmapMemory patch honestly, although I confirmed these worked correctly visually. I will have to do them before submitting CQ.

Comment 8 by hiroh@chromium.org, Jul 7 2017

Today, I finally fix the bug in JDA.
The key is fourcc.
In JDA, fourcc is decided within vaEndPicture.
In the my previous implementation, VASurface called vaDerivePicture to obtatin its memory usage.
The wrong fourcc was suggested in the function.
If wrong fourcc is used, the size of allocated memory get different.
Therefore, the error causes vaGetImage.
The right fourcc is decided after EndPicture.
Therefore, I make SurfaceSize call after calling EndPicture in JDA.

I don't know this is a valid way to fix this bug. Perhaps there is a way to find the correct fourcc before calling EndPicture.
Additionally, I am honestly unsure why no error happened in VDA/VEA.
I will need to investigate them next week and blush up the code more. 

Comment 9 by hiroh@chromium.org, Jul 14 2017

I improved VideoMemoryTracker, basing on reviews from primiano@.
Regrading vaSurface tracking class, Intel deveopers introduced the way to decide the right appropriate fourcc before vaCreateSurfaces() is called.
I adopt the method to VA JDA. Note that such fourcc setting is unnecessary to VEA and VDA.

I wrote comments in the code and confirmed VDA/VDA/JDA unittests were passed and memory-infra worked correctlly in my current implementation.
So I am ready to upload the CL now.
posciak@ will kindly review the CL about V4L2 MMAP memory in few working days.
Because the CL about VASurface will be modified reacting the review, I will upload it after approving the CL about V4L2 MMap memory.

Comment 10 by hiroh@chromium.org, Jul 18 2017

The description shown in memory-infra should include blob to identify which video uses the memory.
Such additional information needs to be passed through VDA/VEA/JDA::Config class.
This issue includes the task and it should be done after all the video allocated memory are supported.
Labels: -videoshortlist
Components: Internals>GPU>Video
Labels: OS-Chrome
Blockedon: 909561
Project Member

Comment 14 by bugdroid1@chromium.org, Nov 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e91e25383686a204378a3fb04f20e1c33db7261b

commit e91e25383686a204378a3fb04f20e1c33db7261b
Author: Miguel Casas <mcasas@chromium.org>
Date: Fri Nov 30 22:26:34 2018

Vaapi decoder: use TRACE_COUNTER_ID2 for "Vaapi frames at client"

This CL addresses two issues:
- Extends "Vaapi frames at client" to show used vs available, where ToT
 only shows the currently used ones, without any reference to its max.
- TRACE_COUNTER1/2 is global per-process, which means that when there
 are multiple Vaapi decoders working, each will update the counter as if
 in isolation, rendering the measurement meaningless. This CL uses
 TRACE_COUNTER_ID2 to add an id to each counter, allowing for counter
 separation [1].

[1] https://i.imgur.com/qq60Xqw.png (https://imgur.com/a/AOcwh78)

Bug: 909926, 721674
Change-Id: If7089d8e9ca614912b3965eb46dc552b8c63928f
Reviewed-on: https://chromium-review.googlesource.com/c/1356884
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Reviewed-by: Daniele Castagna <dcastagna@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612819}
[modify] https://crrev.com/e91e25383686a204378a3fb04f20e1c33db7261b/media/gpu/vaapi/vaapi_video_decode_accelerator.cc

Project Member

Comment 15 by bugdroid1@chromium.org, Dec 3

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4ac08d1e7464017b01dadc237a253114ff62c3d9

commit 4ac08d1e7464017b01dadc237a253114ff62c3d9
Author: Miguel Casas <mcasas@chromium.org>
Date: Mon Dec 03 15:15:02 2018

Vaapi: remove debug var |num_frames_at_client_|

This CL removes the debug variable |num_frames_at_client_| since it's
always equal to the difference between pictures_.size() and
available_picture_buffers_.size().

Test: Checked in crosvideo and v_d_a_unittests that the values are ==.
Bug: 721674
Change-Id: I946192bde8368210c4b65cf0084c34ee86ea3e62
Reviewed-on: https://chromium-review.googlesource.com/c/1357554
Reviewed-by: Hirokazu Honda <hiroh@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613089}
[modify] https://crrev.com/4ac08d1e7464017b01dadc237a253114ff62c3d9/media/gpu/vaapi/vaapi_video_decode_accelerator.cc
[modify] https://crrev.com/4ac08d1e7464017b01dadc237a253114ff62c3d9/media/gpu/vaapi/vaapi_video_decode_accelerator.h

Blockedon: 916807
Project Member

Comment 17 by bugdroid1@chromium.org, Jan 17 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ca3e63ed304404342e689532cc0d2148a57767c1

commit ca3e63ed304404342e689532cc0d2148a57767c1
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Thu Jan 17 10:25:23 2019

media/gpu/v4l2: Make V4L2VDA a MemoryDumpProvider

This makes V4L2VideoDecodeAccelerator a MemoryDumpProvider. Thanks to it, we can
see the memory usage of V4L2VDA in chrome://tracing in enabling memory-infra.
V4L2VDA reports the following memory usage as its usage;
* the size of MMAP buffers in VIDIOC_OUTPUT queue,
* the size of MMAP buffers in VIDIOC_CAPTURE queue,
and also reports the additional information,
* the number of any memory type buffers in VIDIOC_OUTPUT queue,
* the memory type of buffers in VIDIOC_OUTPUT queue,
* the number of any memory type buffers in VIDIOC_CAPTURE queue, and
* the memory type of buffers in VIDIOC_CAPTURE queue.

Screen shot of chrome://tracing: https://imgur.com/a/Wc97koz

Bug: 721674
Test: chrome://tracing memory usage of V4L2VDA when video plays on hana

Change-Id: I9227ccf3fab821341dac14d72a3fdc40f40c5afd
Reviewed-on: https://chromium-review.googlesource.com/c/1390780
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623630}
[modify] https://crrev.com/ca3e63ed304404342e689532cc0d2148a57767c1/media/gpu/v4l2/v4l2_device.cc
[modify] https://crrev.com/ca3e63ed304404342e689532cc0d2148a57767c1/media/gpu/v4l2/v4l2_device.h
[modify] https://crrev.com/ca3e63ed304404342e689532cc0d2148a57767c1/media/gpu/v4l2/v4l2_video_decode_accelerator.cc
[modify] https://crrev.com/ca3e63ed304404342e689532cc0d2148a57767c1/media/gpu/v4l2/v4l2_video_decode_accelerator.h

Project Member

Comment 18 by bugdroid1@chromium.org, Jan 21 (2 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab1acdd9bc3967a1f3c9e7870ff72120cd92c189

commit ab1acdd9bc3967a1f3c9e7870ff72120cd92c189
Author: Hirokazu Honda <hiroh@chromium.org>
Date: Mon Jan 21 10:57:08 2019

media/gpu/v4l2: Make V4L2SVDA a MemoryDumpProvider

This makes V4L2SliceVideoDecodeAccelerator a MemoryDumpProvider. Thanks to it,
we can see the memory usage of V4L2SVDA in chrome://tracing in enabling
memory-infra. V4L2SVDA reports the following memory usage as its usage;
* the number of buffers in VIDIOC_OUTPUT queue,
* the memory type of buffers in VIDIOC_OUTPUT queue,
* the number of buffers in VIDIOC_CAPTURE queue, and
* the memory type of buffers in VIDIOC_CAPTURE queue.

Screen shot of chrome://tracing: https://imgur.com/a/H1FxH2P

Bug: 721674
Test: chrome://tracing memory usage of V4L2SVDA when video plays on kevin

Change-Id: I61325a6c34d5b449ac1e580dbdb01ba2cbfd10fb
Reviewed-on: https://chromium-review.googlesource.com/c/1392694
Reviewed-by: Alexandre Courbot <acourbot@chromium.org>
Commit-Queue: Hirokazu Honda <hiroh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624552}
[modify] https://crrev.com/ab1acdd9bc3967a1f3c9e7870ff72120cd92c189/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.cc
[modify] https://crrev.com/ab1acdd9bc3967a1f3c9e7870ff72120cd92c189/media/gpu/v4l2/v4l2_slice_video_decode_accelerator.h

Sign in to add a comment