| Message ID | CAJ1myNR7stUdFmnAnnWLdE008p5_BvXkHnr7au+TH3HW183X1w@mail.gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | regarding commit "v4l2-ctl: get fmt after source change" in v4l-utils repository | expand |
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();
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(); >
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
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> 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