V4L2SVDA/VAAPIVDA: use visible size from decoder and pass to client

media/gpu/vaapi_video_decode_accelerator.cc

Index: media/gpu/vaapi_video_decode_accelerator.cc
diff --git a/media/gpu/vaapi_video_decode_accelerator.cc b/media/gpu/vaapi_video_decode_accelerator.cc
index 9c6897fa15abd9ac8c453bfb94953f9328bed059..49fb45c44b27cb3c582c8ab1141af2811ca98420 100644
--- a/media/gpu/vaapi_video_decode_accelerator.cc
+++ b/media/gpu/vaapi_video_decode_accelerator.cc
@@ -427,14 +427,13 @@ void VaapiVideoDecodeAccelerator::OutputPicture(
   // Notify the client a picture is ready to be displayed.
   ++num_frames_at_client_;
   TRACE_COUNTER1("Video Decoder", "Textures at client", num_frames_at_client_);
-  DVLOG(4) << "Notifying output picture id " << output_id
-           << " for input " << input_id << " is ready";
-  // TODO(posciak): Use visible size from decoder here instead
-  // (crbug.com/402760). Passing (0, 0) results in the client using the
-  // visible size extracted from the container instead.
+  gfx::Rect visible_rect = decoder_->GetVisibleRect();

[Owen Lin, 2017/06/07 06:11:00]

I am afraid you cannot get the visible size here. The decoder may already saw a
resolution change and switch to a new size.

I would suggest we add a visible_size in H264Picture and set it in the decoders.

[johnylin1, 2017/06/07 15:38:37]

I think it would be better to put visible_size in DecodeSurface.

[Pawel Osciak, 2017/06/08 04:58:24]

Could you explain your reasoning please?

Owen is right, GetVisibleRect() would only be valid at the time of decoding the
particular picture, not at output time.

Moreover, it would be symmetrical to have the visible rect information somehow
contained within all the *Picture classes. For VP9, it's directly in the
Vp9FrameHeader, because the header syntax element contains it explicitly. For
H264, it's calculated from SPS, so it should probably be in H264Picture class
itself, which contains calculated values for each picture. For VP8, it's also
not in the header, so similarly we could put it in VP8Picture as well.

[Owen Lin, 2017/06/09 02:03:17]

I understand why we need this in DecodeSurface. H264Picture is not available in
OutputSurface.

Even though, adding visible size to H264Picture is still better than adding it
to decoders. From the viewpoint of a decoder, the visible size is not
consistent. It is hard to define what GetVisibleSize() returns. (Or you need
document it as only valid at ....) But adding it to H264Picture and set it
insides decoder makes things clear.

We just pass this info from H264Picture to DecodeSurface.

[johnylin1, 2017/06/12 13:41:53]

My thought is same as Owen, we need to have visible_rect in DecodeSurface,
because H264Picture is not available in VDA.

After more tracings, I think maybe we could set visible_rect into H264Picture
when it has been created (same as VP8, VP9). Please see my new modifications on
h264_decoder.cc

+  DVLOG(4) << "Notifying output picture id " << output_id << " for input "
+           << input_id
+           << " is ready. visible rect: " << visible_rect.ToString();
   // TODO(hubbe): Use the correct color space.  http://crbug.com/647725
   if (client_)
-    client_->PictureReady(Picture(output_id, input_id, gfx::Rect(0, 0),
+    client_->PictureReady(Picture(output_id, input_id, visible_rect,
                                   gfx::ColorSpace(), picture->AllowOverlay()));
 }