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

media/gpu/h264_decoder.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <algorithm>
 #include <limits>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/callback_helpers.h"
@@ skipping 1075 matching lines @@
     DVLOG(1) << "frame_mbs_only_flag != 1 not supported";
     return false;
   }
 
   gfx::Size new_pic_size = sps->GetCodedSize().value_or(gfx::Size());
   if (new_pic_size.IsEmpty()) {
     DVLOG(1) << "Invalid picture size";
     return false;
   }
 
+  gfx::Rect new_visible_rect = sps->GetVisibleRect().value_or(gfx::Rect());
+  if (new_visible_rect.IsEmpty()) {

[Owen Lin, 2017/06/07 06:10:59]

I think we shouldn't fail in the case. We can still try to use the coded size. 

Could we still follow the convention that empty rect means information not
available?

[kcwu, 2017/06/07 07:15:27]

Maybe check what spec say how to handle invalid size? Anyway, add warning log if
you don't return failure.

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

In H264Parser::GetVisibleRect() it already checked validity and return
base::nullopt if invlaid. That is, new_visible_rect is empty.

+    DVLOG(1) << "Invalid visible rect";
+    return false;
+  }
+
   int width_mb = new_pic_size.width() / 16;
   int height_mb = new_pic_size.height() / 16;
 
   // Verify that the values are not too large before multiplying.
   if (std::numeric_limits<int>::max() / width_mb < height_mb) {
     DVLOG(1) << "Picture size is too big: " << new_pic_size.ToString();
     return false;
   }
 
   int level = sps->level_idc;
   int max_dpb_mbs = LevelToMaxDpbMbs(level);
   if (max_dpb_mbs == 0)
     return false;
 
   size_t max_dpb_size = std::min(max_dpb_mbs / (width_mb * height_mb),
                                  static_cast<int>(H264DPB::kDPBMaxSize));
   if (max_dpb_size == 0) {
     DVLOG(1) << "Invalid DPB Size";
     return false;
   }
 
-  if ((pic_size_ != new_pic_size) || (dpb_.max_num_pics() != max_dpb_size)) {
+  if ((pic_size_ != new_pic_size) || (visible_rect_ != new_visible_rect) ||

[Owen Lin, 2017/06/07 06:10:59]

We don't care about visible rect change here. Maybe just assign to visible_rect_
in l.1096.

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

Done.

+      (dpb_.max_num_pics() != max_dpb_size)) {
     if (!Flush())
       return false;
     DVLOG(1) << "Codec level: " << level << ", DPB size: " << max_dpb_size
-             << ", Picture size: " << new_pic_size.ToString();
+             << ", Picture size: " << new_pic_size.ToString()
+             << ", Visible rect: " << new_visible_rect.ToString();
     *need_new_buffers = true;
     pic_size_ = new_pic_size;
+    visible_rect_ = new_visible_rect;
     dpb_.set_max_num_pics(max_dpb_size);
   }
 
   if (!UpdateMaxNumReorderFrames(sps))
     return false;
   DVLOG(1) << "max_num_reorder_frames: " << max_num_reorder_frames_;
 
   return true;
 }
 
@@ skipping 288 matching lines @@
 
     DVLOG(4) << "NALU done";
     curr_nalu_.reset();
   }
 }
 
 gfx::Size H264Decoder::GetPicSize() const {
   return pic_size_;
 }
 
+gfx::Rect H264Decoder::GetVisibleRect() const {
+  return visible_rect_;
+}
+
 size_t H264Decoder::GetRequiredNumOfPictures() const {
   return dpb_.max_num_pics() + kPicsInPipeline;
 }
 
 }  // namespace media