Hi Philipp,
On Monday 25 August 2014 17:41:17 Philipp Zabel wrote:
> Am Montag, den 25.08.2014, 17:13 +0200 schrieb Laurent Pinchart:
> [...]
>
> >>>> +static inline unsigned int v4l2_sizeimage(const struct v4l2_pixfmt
> >>>> *fmt,
> >>>> + unsigned int width,
> >>>> + unsigned int height)
> >>>> +{
> >>>> + return width * height * fmt->bpp_image / 8;
> >>>
> >>> Ditto: return height * v4l2_bytesperline(fmt, width);
> >>
> >> I can't use v4l2_bytesperline because that might be zero for macroblock
> >> tiled formats and uses the wrong bpp value for planar formats with
> >> subsampled chroma.
> >>
> >> This nearly works:
> >> return height * DIV_ROUND_UP(width * fmt->bpp_image, 8)
> >
> > Isn't that exactly height * v4l2_bytesperline(fmt, width) ?
>
> Only if bpp_image == bpp_line ...
Oops, I've missed that subtle difference, sorry.
> >> For the planar 4:2:0 subsampled formats and Y41P (bpp_image == 12),
> >> width has to be even, so that is ok.
> >> Most other formats have a bpp_image that is divisible by 8, but there
> >> are the 4:1:0 subsampled formats (bpp_image == 9). Those would round up
> >> width to a multiple of eight, even though it only has to be a multiple
> >>
> >> of four. I'd fall back to:
> >> if (fmt->bpp_image == fmt->bpp_line) {
> >>
> >> return height * DIV_ROUND_UP(width * fmt->bpp_image, 8);
>
> ... as is the case here. I'll use v4l2_bytesperline, then.
>
> >> } else {
> >>
> >> /* we know that v4l2_bytesperline doesn't round for planar */
> >> return height * width * fmt->bpp_image / 8;
> >>
> >> }
> >
> > Isn't that growing slightly too big for an inline function ?
>
> Yes, I think this is slightly over the edge. Is room for a function to
> accompany the preexisting v4l2_fill_pix_format (say,
> v4l2_fill_pix_format_size) to set both the bytesperline and sizeimage
> values in a struct v4l2_pix_format?
That sounds sensible to me, provided it would be used by drivers of course. I
wouldn't remove v4l2_bytesperline() and v4l2_sizeimage(), as the values might
be needed by drivers in places where a v4l2_pix_format structure isn't
available.
> Also, is anybody bothered by the v4l2_pix_format / v4l2_pixfmt
> similarity in name?
How about renaming v4l2_pixfmt to v4l2_pix_format_info ?
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
