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

Issue 900302 link

Starred by 1 user
Status: Fixed
Owner:
dalecur...@chromium.org
Closed: Nov 5
Cc:
█ fgalligan@chromium.org
dalecur...@chromium.org
█ tomfinegan@chromium.org
jzern@chromium.org
█ yaowu@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Casting int64_t to void* may truncate the value in 32-bit builds

Project Member Reported by wtc@google.com, Oct 30 
In media/filters/aom_video_decoder.cc, we pass an int64_t timestamp
as the user_priv argument to aom_codec_decode(). This requires casting
an int64_t value to a void* pointer:

bool AomVideoDecoder::DecodeBuffer(const DecoderBuffer* buffer) {
  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
  DCHECK(!buffer->end_of_stream());

  if (aom_codec_decode(
          aom_decoder_.get(), buffer->data(), buffer->data_size(),
          reinterpret_cast<void*>(buffer->timestamp().InMicroseconds())) !=
      AOM_CODEC_OK) {

In a 32-bit build, a void* pointer is 32 bits, so the cast to void* may
potentially truncate the int64_t timestamp.

Dale, is it possible for buffer->timestamp().InMicroseconds() to be
greater than or equal to 2^32?

Do you have any suggestions on how to fix this? Thanks.

Comment 1 by dalecur...@chromium.org, Oct 30 

Yes, I think we can just drop this in favor of a  base::circular_deque of the timestamps if the aom iterator is expected to return multiple frames. pts/dts are the same for av1 streams, so deque should be fine.

Comment 3 by dalecur...@chromium.org, Nov 2

Owner: dalecur...@chromium.org
Status: Started (was: Untriaged)
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 3 

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

commit 68a3b425ac9cc520cefb62a2adf673c8b94d8bb6
Author: Dale Curtis <dalecurtis@chromium.org>
Date: Sat Nov 03 02:01:51 2018

Switch AomVideoDecoder to zero copy now that libaom fixes are in.

libaom has fixed all the framebuffer issues, so we should now be
able to use these. All tests pass and YouTube videos look good.
Seems only a minor memory savings ~2mb on a 480p clip.

This also resolves an issue with clipping of 64-bit timestamps
passed in as a void* (which might be 32-bit on some platforms).

BUG= 867613 , 900302 
TEST=existing tests all still pass.

Change-Id: I83af8ecd867e13741f27f5225316b62f31981562
Reviewed-on: https://chromium-review.googlesource.com/c/1316408
Commit-Queue: Dale Curtis <dalecurtis@chromium.org>
Reviewed-by: Thomas Guilbert <tguilbert@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605129}
[modify] https://crrev.com/68a3b425ac9cc520cefb62a2adf673c8b94d8bb6/media/filters/aom_video_decoder.cc
[modify] https://crrev.com/68a3b425ac9cc520cefb62a2adf673c8b94d8bb6/media/filters/aom_video_decoder.h

Comment 5 by dalecur...@chromium.org, Nov 5

Status: Fixed (was: Started)
►

Sign in to add a comment