[Libva] [PATCH 2/3] Encode: Don't mess up the index handling of packed raw data with packed slice header

[Sreerenj]

On 29.05.2014 03:42, Zhao, Yakui wrote:
> On Wed, 2014-05-28 at 15:02 -0600, sreerenj.balachandran at intel.com
> wrote:
>> From: Sreerenj Balachandran <sreerenj.balachandran at 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 ?


>
>      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?



> 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.