On 30.05.2014 11:48, Zhao, Yakui wrote:
On Fri, 2014-05-30 at 11:06 +0300, Sreerenj wrote:
On 29.05.2014 03:42, Zhao, Yakui wrote:
On Wed, 2014-05-28 at 15:02 -0600, sreerenj.balachand...@intel.com
wrote:
From: Sreerenj Balachandran <sreerenj.balachand...@intel.com>

Avoid storing packed slice header index as packed raw data index.
This patch is a preparation for submitting all the packed slice
headers as a group , instead of pairing with VAEncSliceParameterBuffer in
multi slice per frame encoding. If the user only passes the packed
slice_header data for one slice before VAEncSliceParameterbuffer
one by one, it still can work without this patch.
Hi, Sreerenj

      Thanks for your patch.
      But I have two concerns about this patch and following patch(grouped
submission of packed_slice header)
      a. After this patch, it won't work when passing the packed
rawdata/slice header like the below order for one slice(Of course they
are passed before VAEncSliceParameterBuffer).
         >packed rawdata 0, slice_header , packed rawdata 1;
In such case the packed rawdata 1 won't be inserted as expected.

         Without this patch, it will firstly insert packed rawdata 0/1 and
then insert the slice_header.

         If so, it will be better that a new structure is added to store
the packed slice_header data.

I didn't understand the issue here. With this patch i have not changed
any logic i guess. isn't it? The index value num_slice_params_ext will
get incremented only when the driver receives
the VAEncSliceParameterBuffer. If I understood correctly you were
talking about a case having two packed_raw_data and one slice_header
before the submission of VAEncSliceParameterBuffer.
I haven't seen any issue. The packed_raw_data_0 will inserted to
slice_rawdata_index[0] then the packed_slice_header will be inserted to
slice_header_index[0] , finally the slice_raw_data_1 will
be inserted to slice_rawdata_index[1].  isn't it ?
But the current logic is related with the function of
intel_avc_slice_insert_packed_data.  Now the packed rawdata &
slice_header data are tracked by using the
packed_header_params_ext/packed_header_data_ext.(The packed slice_header
data is one specific type of packed rawdata. But it still increases the
slice_rawdata_count for the given slice).

The below is the rough flowchart about how to insert the packed data.
    1. use the slice_rawdata_index/count to enumerate and insert the
corresponding packed_data. But the slice_header is skipped in this step,
which is to assure that slice_header is lastly inserted for one slice.
    2. use the slice_header_index to insert the packed slice_header data.

After this patch, the slice_rawdata_count is different for the below
sequence of "packed rawdata 0, packed slice_header 0, packed rawdata 1".


Okay I understood the issue. Thanks :) But this is really misleading . It would be good if you can write some comments at least. Because the name raw_data_count implies the number of raw_datas and usually some one who read the code won't think it includes the count of some thing else too :)





      b. The next patch will try to group the submission of packed
slice_header data for multi-slices. If the frame is divided into four
slices and three slice header data are passed, how will it be handled?
And how to decide which slice header data is inserted for one slice? As
you know, the current mechanism is that the slice_header data is
optional.(That is to say: For one slice the driver will firstly try to
insert the slice_header data passed from user. If it doesn't exist, it
will generate the data by itself.) So the submission of slice_header
data is also based on the VAEncSliceParameterBuffer delimeter.


I too thought about this case before the implementation.AFAIK we are not
supposed to impose any ordering rule for packed_slice_headers, since as
per the va specification the only packed_header having order restriction
is  Packed Raw Data. So the scenario should work like this:  If the user
configured the driver to work with packed_slice_header then it is the
responsibility of user to submit all slice_headers. On the other
hand if the user have created the VAConfig with out PACKED_SLICE_HEADER
support then he is not supposed to provide any packed_slice_header.
Which means the user is not allowed
to use a mixed mode like " four slices and three packed slice headers".
What do you think?
This proposal of the PACKED_SLICE_HEADER restriction is OK to me.

But not sure whether the current implementation is still acceptable to
you and meet with your requirement. (The packed slice_header for every
slice is still based on VAEncSliceParameterBuffer delimeter. Not use the
grouped submission of all slice_headers before
VAEncSliceParameterBuffer).


So options here:
option1: restrict the submission of packed_slice_headers such that it should be aligned with VAEncSliceParamerterBuffer and add it to the va specification. Which mean no support for grouped submission of packed_slice_headers for each frame.
option2: add support for grouped slice headers . No addition to the va spec.

I guess you are preferring to follow option1.  right?



Thanks.
     Yakui



Thanks.
      Yakui

---
   src/i965_drv_video.c | 7 +++++--
   1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/i965_drv_video.c b/src/i965_drv_video.c
index 44da864..4854a62 100755
--- a/src/i965_drv_video.c
+++ b/src/i965_drv_video.c
@@ -2341,8 +2341,7 @@ i965_encoder_render_picture(VADriverContextP ctx,
                   vaStatus = VA_STATUS_ERROR_INVALID_PARAMETER;
                   return vaStatus;
               }
-            if (encode->last_packed_header_type == VAEncPackedHeaderRawData ||
-                encode->last_packed_header_type == VAEncPackedHeaderSlice) {
+            if (encode->last_packed_header_type == VAEncPackedHeaderRawData) {
                   vaStatus = I965_RENDER_ENCODE_BUFFER(packed_header_data_ext);
                   if (vaStatus == VA_STATUS_SUCCESS) {
                       /* store the first index of the packed header data for 
current slice */
@@ -2351,6 +2350,10 @@ i965_encoder_render_picture(VADriverContextP ctx,
                                SLICE_PACKED_DATA_INDEX_TYPE | 
(encode->num_packed_header_data_ext - 1);
                       }
                       
encode->slice_rawdata_count[encode->num_slice_params_ext]++;
+                }
+           } else if (encode->last_packed_header_type == 
VAEncPackedHeaderSlice) {
+                vaStatus = I965_RENDER_ENCODE_BUFFER(packed_header_data_ext);
+                if (vaStatus == VA_STATUS_SUCCESS) {
                       if (encode->last_packed_header_type == 
VAEncPackedHeaderSlice) {
                           if 
(encode->slice_header_index[encode->num_slice_params_ext] == 0) {
                               
encode->slice_header_index[encode->num_slice_params_ext] =


--
Thanks
Sree

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki Business Identity Code: 0357606 - 4 Domiciled in Helsinki
This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
Libva mailing list
Libva@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libva

Reply via email to