[Bug 743186] v4l2object: set colorspace in caps for capture devices

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=743186
  GStreamer | gst-plugins-good | git master

--- Comment #3 from Aurélien Zanelli <aurelien.zanelli at parrot.com> 2015-01-19 17:02:09 UTC ---
(In reply to comment #2)
> Review of attachment 294894 [details]:
> 
> Few comments, the last one is mostly a reflection, but I'm not 100% sure it
> matter.
> 
> ::: sys/v4l2/gstv4l2object.c
> @@ +1675,3 @@
> +      break;
> +    default:
> +      GST_ERROR ("Unknown enum v4l2_colorspace %d", colorspace);
> 
> I think some driver don't support it, hence will set 0. I think we should
> handle that case without spamming the logs. I already have two spammer to fix 
> now ;-P Also, see the guessing code we do in the opposite direction.

If the driver doesn't set the colorspace, I think we can just leave the caps
with no colorimetry since the guess could be really wrong and we could live
without colorimetry. 
In the opposite direction, we guess the colorspace as a last resort because the
application must set it according to the spec.

However, I agree with the fact that I'm maybe too aggressive with an error :)
for a thing we can live without.
So what do you think of a warning or debug message instead (warning will also
spam)

> 
> @@ +1791,3 @@
> +    fmt.fmt.pix.height = height;
> +    fmt.fmt.pix.pixelformat = pixelformat;
> +  }
> 
> The if case is not needed, this part of the structure is that same for mplane.
Agreed, I will merge it.

> 
> @@ +2308,3 @@
>          pixelformat);
>      gst_v4l2_object_add_aspect_ratio (v4l2object, tmp);
> +    gst_v4l2_object_add_colorspace (v4l2object, tmp, max_w, max_h,
> pixelformat);
> 
> I think this is a case where just checking the max may really lead to wrong
> result. We probably want to check min and max, if if different, put both in the
> caps.

Not a bad reflection, I don't know if the size matter or not, it could depend
of the driver I think. But in this case we would know for sure the colorspace
when the format is negotiatied, ie after final S_FMT.
I think if some drivers do this, checking min and max could not be enough. For
instance, I think of a device which can output SD with associated colorspace
smtpe170m, HD in rec709 and UHD in bt2020.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.