Re: [PATCH 2/2] vicodec: set state->info before calling the encode/decode funcs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/10/2018 05:37 PM, Ezequiel Garcia wrote:
> On Mon, 2018-09-10 at 17:00 +0200, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>>
>> state->info was NULL since I completely forgot to set state->info.
>> Oops.
>>
>> Reported-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
> 
> For both patches:
> 
> Tested-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxx>
> 
> With these changes, now this gstreamer pipeline no longer
> crashes:
> 
> gst-launch-1.0 -v videotestsrc num-buffers=30 ! video/x-raw,width=1280,height=720 ! v4l2fwhtenc capture-io-mode=mmap output-io-mode=mmap ! v4l2fwhtdec
> capture-io-mode=mmap output-io-mode=mmap ! fakesink
> 
> A few things:
> 
>   * You now need to mark "[PATCH] vicodec: fix sparse warning" as invalid.

I'll wait for that to be merged (it's already in a pending pull request), then
rework this patch and add your other patches for a new pull request.

>   * v4l2fwhtenc/v4l2fwhtdec elements are not upstream yet.
>   * Gstreamer doesn't end properly; and it seems to negotiate
>     different sizes for encoded and decoded unless explicitly set.

As mentioned before, vicodec isn't fully compliant with the upcoming
codec spec, and is also missing certain features (selection support,
support for custom bytesperline values, padding, midstream resolution
changes). Patches are welcome.

If you are working on gstreamer elements for this codec, then it would
be great if you could look at making the driver compliant. I have no
plans to work on vicodec until that codec spec is fully finalized, so
you can go ahead with that if you want to.

Would be really nice, and after all, that's why I wrote vicodec!

Regards,

	Hans

> 
> Thanks!
> 
>>  drivers/media/platform/vicodec/vicodec-core.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c
>> index fdd77441a47b..5d42a8414283 100644
>> --- a/drivers/media/platform/vicodec/vicodec-core.c
>> +++ b/drivers/media/platform/vicodec/vicodec-core.c
>> @@ -176,12 +176,15 @@ static int device_process(struct vicodec_ctx *ctx,
>>  	}
>>  
>>  	if (ctx->is_enc) {
>> -		unsigned int size = v4l2_fwht_encode(state, p_in, p_out);
>> -
>> -		vb2_set_plane_payload(&out_vb->vb2_buf, 0, size);
>> +		state->info = q_out->info;
>> +		ret = v4l2_fwht_encode(state, p_in, p_out);
>> +		if (ret < 0)
>> +			return ret;
>> +		vb2_set_plane_payload(&out_vb->vb2_buf, 0, ret);
>>  	} else {
>> +		state->info = q_cap->info;
>>  		ret = v4l2_fwht_decode(state, p_in, p_out);
>> -		if (ret)
>> +		if (ret < 0)
>>  			return ret;
>>  		vb2_set_plane_payload(&out_vb->vb2_buf, 0, q_cap->sizeimage);
>>  	}
> 




[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]

  Powered by Linux