linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sergey Zakharchenko <doublef.mobile@gmail.com>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Sergey Zakharchenko <szakharchenko@digital-loggers.com>,
	linux-media@vger.kernel.org,
	Mauro Carvalho Chehab <mchehab@kernel.org>,
	Martin Bodo <martin@digital-loggers.com>,
	"Logan, Peter" <peter.logan@intel.com>,
	Auke Kok <auke-jan.h.kok@intel.com>
Subject: Re: [PATCH v2] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value
Date: Wed, 2 Oct 2019 21:15:45 +0400	[thread overview]
Message-ID: <CADYCxoNTBmX8RYf18xmX0B8xD_LvNJbZ5buyAgo5E9v14B3S_A@mail.gmail.com> (raw)
In-Reply-To: <CADYCxoOzwRQctuVaqEx40WgFLo4PY0sV4R11r+pn+Wu0jLzPng@mail.gmail.com>

Sergey Zakharchenko <doublef.mobile@gmail.com>:
> Laurent Pinchart <laurent.pinchart@ideasonboard.com>:
> > Do you think it would make sense to do this by default, without
> > requiring a quirk ? Or are there cases where this calculation would lead
> > to incorrect results while the bpp reported by the camera would be right
> > ?
>
> The loop is a simplified version of the v4l2_fill_pixfmt() loop. The
> calculation might need some checking, and might be invalid, in case
> block_w/block_h format fields are significant (not 0 and not 1),
> because then effective bits-per-pixel would seemingly be fractional,
> and depend on the image dimensions if they weren't aligned; however I
> see no formats using the block_w/block_h fields defined so far.

It's likely possible to directly replace the bpp-using computation in
https://github.com/torvalds/linux/blob/2874c5fd284268364ece81a7bd936f3c8168e567/drivers/media/usb/uvc/uvc_driver.c#L636
with a call to v4l2_fill_pixfmt() and the sizeimage it returns.
However, bpp is used elsewhere, and it's hard to tell what it should
be taken to be to in the hypothetical exotic cases I'm considering, so
I'm reluctant to go that route.

I'm going to send v3 in an hour unless there are other suggestions.

Best regards,

-- 
Sergey Zakharchenko
Digital Loggers, Inc.

  reply	other threads:[~2019-10-02 17:15 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-02  8:47 [PATCH] media: uvcvideo: Add a quirk to force GEO GC6500 Camera bits-per-pixel value Sergey Zakharchenko
2019-10-02 12:00 ` kbuild test robot
2019-10-02 13:01   ` [PATCH v2] " Sergey Zakharchenko
2019-10-02 14:08     ` Laurent Pinchart
2019-10-02 14:54       ` Sergey Zakharchenko
2019-10-02 17:15         ` Sergey Zakharchenko [this message]
2019-10-02 19:08           ` Laurent Pinchart
2019-10-02 19:29             ` Sergey Zakharchenko
2019-10-03  9:31               ` [PATCH v3] " Sergey Zakharchenko
2019-12-18 18:00                 ` Laurent Pinchart
2019-12-18 18:46                   ` Sergey Zakharchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CADYCxoNTBmX8RYf18xmX0B8xD_LvNJbZ5buyAgo5E9v14B3S_A@mail.gmail.com \
    --to=doublef.mobile@gmail.com \
    --cc=auke-jan.h.kok@intel.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-media@vger.kernel.org \
    --cc=martin@digital-loggers.com \
    --cc=mchehab@kernel.org \
    --cc=peter.logan@intel.com \
    --cc=szakharchenko@digital-loggers.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).