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

Issue 899916 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Chromium does not need to pass a user_priv argument to vpx_codec_decode and aom_codec_decode

Project Member Reported by wtc@google.com, Oct 29

Issue description

The vpx_codec_decode() and aom_codec_decode() functions have a
void *user_priv input parameter. This input argument is copied to
the user_priv field of the vpx_image_t or aom_image_t returned by
vpx_codec_get_frame() or aom_codec_get_frame() function.

Chromium passes either the address of a timestamp local variable or
the a timestamp cast to a void* pointer as the user_priv argument to
vpx_codec_decode() and aom_codec_decode(). When I inspected how the
user_priv value is used, I found that it does nothing but to check
that libvpx or libaom copies that value to vpx_image_t or aom_image_t
correctly. Therefore, Chromium can just pass nullptr as the user_priv
argument, like all other users of libvpx and libaom that I've
inspected.

If Chromium cannot pass nullptr as the user_priv argument, that will
imply vpxdec.c and aomdec.c, the sample decoder program of libvpx and
libaom, have a bug, because both of them pass nullptr as the user_priv
argument. That will be very bad.

I will upload a CL shortly, which will show the relevant code in
chromium/src/media/filters/.

Note: Another potential issue I found is that Chromium does not call
vpx_codec_decode() and aom_codec_decode() with data=nullptr and
data_sz=0 to flush the decoder. I don't know if this is a problem.
 
This is the CL that illustrates this issue:

https://chromium-review.googlesource.com/c/chromium/src/+/1306413

Please let me know if I missed something. Thank you!
Cc: tomfinegan@chromium.org
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 31

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

commit d299e1bba491c296ab0a1916047c9c7e2646472c
Author: Wan-Teh Chang <wtc@google.com>
Date: Wed Oct 31 01:41:39 2018

Pass nullptr as user_priv to vpx_codec_decode.

Passing the address of a timestamp local variable as the user_priv
argument to vpx_codec_decode() merely verifies that
vpx_codec_get_frame() copies that value to the user_priv field of
vpx_image_t correctly. No other clients of libvpx that I've inspected do
this, including vpxdec.c, the sample decoder program.

BUG= chromium:899916 

Change-Id: I65dfd479fa3634429c4b76285c257aa54d860d81
Reviewed-on: https://chromium-review.googlesource.com/c/1306413
Commit-Queue: Wan-Teh Chang <wtc@google.com>
Reviewed-by: Dale Curtis <dalecurtis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604104}
[modify] https://crrev.com/d299e1bba491c296ab0a1916047c9c7e2646472c/media/filters/vpx_video_decoder.cc

Status: Fixed (was: Untriaged)
See  bug 900302  for the problem with the user_priv argument to aom_codec_decode()
in Chromium and the fix.

Sign in to add a comment