* regarding commit "v4l2-ctl: get fmt after source change" in v4l-utils repository
@ 2019-07-01 10:30 Dafna Hirschfeld
2019-07-10 10:12 ` Hans Verkuil
0 siblings, 1 reply; 3+ messages in thread
From: Dafna Hirschfeld @ 2019-07-01 10:30 UTC (permalink / raw)
To: Hans Verkuil, Linux Media Mailing List
commit 84219e2b5d013709ee5259621715966c46eec746
Author: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Date: Sat Mar 30 14:16:25 2019 +0100
v4l2-ctl: get fmt after source change
When a source change event arrives during decoding get the new
format at that point instead of after restarting streaming.
If there is another source change queued up, then when you call
streamon for CAPTURE again it might send the new source change
event and update the format for that one, so reading the format
after streamon might give the wrong format.
Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
index bb656584..3695a027 100644
--- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
+++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp
@@ -2363,12 +2363,11 @@ static void stateful_m2m(cv4l_fd &fd,
cv4l_queue &in, cv4l_queue &out,
if (in_source_change_event) {
in_source_change_event = false;
last_buffer = false;
+ fd.g_fmt(fmt_in, in.g_type());
if (capture_setup(fd, in, exp_fd_p))
return;
fps_ts[CAP].reset();
fps_ts[OUT].reset();
- fd.g_fmt(fmt_out, out.g_type());
- fd.g_fmt(fmt_in, in.g_type());
Removing those lines cause inconsistency when the user send the wanted
capture pixel format when decoding with the `v4l2-ctl` command. In
this case the value of `-v pixelformat=...` is updated in the kernel
with the capture_setup function but it is not updated in the fmt_in
variable and so the command will try to save the decoded video in a
different format from what is configured in the kernel.
cap_streaming = true;
} else {
break;
Dafna
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: regarding commit "v4l2-ctl: get fmt after source change" in v4l-utils repository 2019-07-01 10:30 regarding commit "v4l2-ctl: get fmt after source change" in v4l-utils repository Dafna Hirschfeld @ 2019-07-10 10:12 ` Hans Verkuil 2019-07-10 12:16 ` Hans Verkuil 0 siblings, 1 reply; 3+ messages in thread From: Hans Verkuil @ 2019-07-10 10:12 UTC (permalink / raw) To: Dafna Hirschfeld, Linux Media Mailing List Hi Dafna, On 7/1/19 12:30 PM, Dafna Hirschfeld wrote: > commit 84219e2b5d013709ee5259621715966c46eec746 > Author: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Date: Sat Mar 30 14:16:25 2019 +0100 > > v4l2-ctl: get fmt after source change > > When a source change event arrives during decoding get the new > format at that point instead of after restarting streaming. > > If there is another source change queued up, then when you call > streamon for CAPTURE again it might send the new source change > event and update the format for that one, so reading the format > after streamon might give the wrong format. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > > diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp > b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp > index bb656584..3695a027 100644 > --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp > +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp > @@ -2363,12 +2363,11 @@ static void stateful_m2m(cv4l_fd &fd, > cv4l_queue &in, cv4l_queue &out, > if (in_source_change_event) { > in_source_change_event = false; > last_buffer = false; > + fd.g_fmt(fmt_in, in.g_type()); > if (capture_setup(fd, in, exp_fd_p)) > return; > fps_ts[CAP].reset(); > fps_ts[OUT].reset(); > - fd.g_fmt(fmt_out, out.g_type()); > - fd.g_fmt(fmt_in, in.g_type()); > Removing those lines cause inconsistency when the user send the wanted > capture pixel format when decoding with the `v4l2-ctl` command. In > this case the value of `-v pixelformat=...` is updated in the kernel > with the capture_setup function but it is not updated in the fmt_in > variable and so the command will try to save the decoded video in a > different format from what is configured in the kernel. > > cap_streaming = true; > } else { > break; > > > Dafna > It looks like you are right. Can you try this v4l2-ctl patch? Regards, Hans [PATCH] v4l2-ctl: let capture_setup return the updated format Just getting the new format after a source change event isn't enough in the case that -v option was specified by the user. Instead let capture_setup optionally return the new format after any '-v' changes are applied. Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> Reported-by: Dafna Hirschfeld <dafna3@gmail.com> --- diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp index 28b2b3b9..470fa766 100644 --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp @@ -2146,7 +2146,7 @@ enum stream_type { OUT, }; -static int capture_setup(cv4l_fd &fd, cv4l_queue &in, cv4l_fd *exp_fd) +static int capture_setup(cv4l_fd &fd, cv4l_queue &in, cv4l_fd *exp_fd, cv4l_fmt *new_fmt = NULL) { if (fd.streamoff(in.g_type())) { fprintf(stderr, "%s: fd.streamoff error\n", __func__); @@ -2168,6 +2168,10 @@ static int capture_setup(cv4l_fd &fd, cv4l_queue &in, cv4l_fd *exp_fd) return -1; } fd.s_fmt(fmt, in.g_type()); + if (new_fmt) + *new_fmt = fmt; + } else if (new_fmt) { + fd.g_fmt(*new_fmt, in.g_type()); } get_cap_compose_rect(fd); @@ -2367,8 +2371,7 @@ static void stateful_m2m(cv4l_fd &fd, cv4l_queue &in, cv4l_queue &out, if (in_source_change_event) { in_source_change_event = false; last_buffer = false; - fd.g_fmt(fmt_in, in.g_type()); - if (capture_setup(fd, in, exp_fd_p)) + if (capture_setup(fd, in, exp_fd_p, &fmt_in)) return; fps_ts[CAP].reset(); fps_ts[OUT].reset(); ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: regarding commit "v4l2-ctl: get fmt after source change" in v4l-utils repository 2019-07-10 10:12 ` Hans Verkuil @ 2019-07-10 12:16 ` Hans Verkuil 0 siblings, 0 replies; 3+ messages in thread From: Hans Verkuil @ 2019-07-10 12:16 UTC (permalink / raw) To: Dafna Hirschfeld, Linux Media Mailing List On 7/10/19 12:12 PM, Hans Verkuil wrote: > Hi Dafna, > > On 7/1/19 12:30 PM, Dafna Hirschfeld wrote: >> commit 84219e2b5d013709ee5259621715966c46eec746 >> Author: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> Date: Sat Mar 30 14:16:25 2019 +0100 >> >> v4l2-ctl: get fmt after source change >> >> When a source change event arrives during decoding get the new >> format at that point instead of after restarting streaming. >> >> If there is another source change queued up, then when you call >> streamon for CAPTURE again it might send the new source change >> event and update the format for that one, so reading the format >> after streamon might give the wrong format. >> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> >> >> diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp >> b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp >> index bb656584..3695a027 100644 >> --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp >> +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp >> @@ -2363,12 +2363,11 @@ static void stateful_m2m(cv4l_fd &fd, >> cv4l_queue &in, cv4l_queue &out, >> if (in_source_change_event) { >> in_source_change_event = false; >> last_buffer = false; >> + fd.g_fmt(fmt_in, in.g_type()); >> if (capture_setup(fd, in, exp_fd_p)) >> return; >> fps_ts[CAP].reset(); >> fps_ts[OUT].reset(); >> - fd.g_fmt(fmt_out, out.g_type()); >> - fd.g_fmt(fmt_in, in.g_type()); >> Removing those lines cause inconsistency when the user send the wanted >> capture pixel format when decoding with the `v4l2-ctl` command. In >> this case the value of `-v pixelformat=...` is updated in the kernel >> with the capture_setup function but it is not updated in the fmt_in >> variable and so the command will try to save the decoded video in a >> different format from what is configured in the kernel. >> >> cap_streaming = true; >> } else { >> break; >> >> >> Dafna >> > > It looks like you are right. Can you try this v4l2-ctl patch? > > Regards, > > Hans > > [PATCH] v4l2-ctl: let capture_setup return the updated format > > Just getting the new format after a source change event isn't > enough in the case that -v option was specified by the user. > > Instead let capture_setup optionally return the new format after > any '-v' changes are applied. > > Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl> > Reported-by: Dafna Hirschfeld <dafna3@gmail.com> I pushed this patch since your test with 'blog.sh' now works with this patch applied. Regards, Hans > --- > diff --git a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp > index 28b2b3b9..470fa766 100644 > --- a/utils/v4l2-ctl/v4l2-ctl-streaming.cpp > +++ b/utils/v4l2-ctl/v4l2-ctl-streaming.cpp > @@ -2146,7 +2146,7 @@ enum stream_type { > OUT, > }; > > -static int capture_setup(cv4l_fd &fd, cv4l_queue &in, cv4l_fd *exp_fd) > +static int capture_setup(cv4l_fd &fd, cv4l_queue &in, cv4l_fd *exp_fd, cv4l_fmt *new_fmt = NULL) > { > if (fd.streamoff(in.g_type())) { > fprintf(stderr, "%s: fd.streamoff error\n", __func__); > @@ -2168,6 +2168,10 @@ static int capture_setup(cv4l_fd &fd, cv4l_queue &in, cv4l_fd *exp_fd) > return -1; > } > fd.s_fmt(fmt, in.g_type()); > + if (new_fmt) > + *new_fmt = fmt; > + } else if (new_fmt) { > + fd.g_fmt(*new_fmt, in.g_type()); > } > get_cap_compose_rect(fd); > > @@ -2367,8 +2371,7 @@ static void stateful_m2m(cv4l_fd &fd, cv4l_queue &in, cv4l_queue &out, > if (in_source_change_event) { > in_source_change_event = false; > last_buffer = false; > - fd.g_fmt(fmt_in, in.g_type()); > - if (capture_setup(fd, in, exp_fd_p)) > + if (capture_setup(fd, in, exp_fd_p, &fmt_in)) > return; > fps_ts[CAP].reset(); > fps_ts[OUT].reset(); > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-07-10 12:16 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-07-01 10:30 regarding commit "v4l2-ctl: get fmt after source change" in v4l-utils repository Dafna Hirschfeld 2019-07-10 10:12 ` Hans Verkuil 2019-07-10 12:16 ` Hans Verkuil
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).