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".
>
>
> >
> > 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).
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] =
> >
>
_______________________________________________
Libva mailing list
Libva@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libva
