[Bug 719359] vp8dec: Doesn't handle changes in resolution
GStreamer (bugzilla.gnome.org)
bugzilla at gnome.org
Tue Feb 25 10:52:48 PST 2014
https://bugzilla.gnome.org/show_bug.cgi?id=719359
GStreamer | gst-plugins-good | 1.0.10
--- Comment #2 from tcdgreenwood at hotmail.com 2014-02-25 19:14:09 UTC ---
(In reply to comment #1)
> Review of attachment 262856 [details]:
>
> Basically good, needs to be applied to vp9dec too which is more or less the
> same code.
>
> ::: ext/vpx/gstvp8dec.c
> @@ +430,2 @@
> g_assert (dec->output_state == NULL);
> + dec->output_state = gst_video_decoder_set_output_state (GST_VIDEO_DECODER
> (dec), GST_VIDEO_FORMAT_I420, stream_info.w, stream_info.h, state); /* Use
> NULL? Why use input_state? */
>
> Please run all this through gst-indent again after your changes, also you
> changed nothing in this line :)
I will remove this bit. I was just going through trying to understand the code
really.
>
> The input state is passed there to copy over things like the pixel-aspect-ratio
> and the framerate
>
> @@ +551,3 @@
> + new_output_state =
> + gst_video_decoder_set_output_state (GST_VIDEO_DECODER (dec),
> + GST_VIDEO_FORMAT_I420, img->d_w, img->d_h, dec->output_state);
>
> Must use the input state here
I used the output state as the basis as the input_state has been put into the
output_state already within open_codec and this code is only used when the size
changes. I wanted to make sure that anything that was done to the output_state
was preserved as otherwise I thought I might trigger a reset of the decoder and
only changes to width/height were applied. Am I guaranteed that nothing else
has been put into the output_state that needs to be preserved? Is there a
guarantee that the input_state will work out to be the same? The input_state
is used in open_codec so any changes in input_state will be preserved there.
Basically I want to ensure that as far as possible the state doesn't get
changed except for width/height at this point - from the docs that seemed the
correct way to go. Are there some guidelines somewhere about what to use?
Perhaps the docs for gst_video_decoder_set_output_state should be changed to
state that the last parameter should always be the input state?
>
> @@ +555,3 @@
> + gst_video_codec_state_unref (dec->output_state);
> + }
> + dec->output_state = new_output_state;
>
> You might want to call gst_video_decoder_negotiate() manually here... not
> really required, but you could give a more useful error message if that fails
> than when allocate_output_frame() below fails.
I noticed that the error message in gstvideodecoder.c wasn't that great, I
actually have changed that code at times to debug better, but I thought it was
better to rely on the shared code than to duplicate the call to negotiate here.
--
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.
More information about the gstreamer-bugs
mailing list