g_return_val_fail() documentations says:
If expr evaluates to FALSE, the current function should be
considered to have undefined behaviour (a programmer error).
The only correct solution to such an error is to change the
module that is calling the current function, so that it avoids
this incorrect call.
So it was missused in a couple parts of the H264 and H265 internal
decoders. This patch changes that to plain conditionals.
Also, it was included a couple code-style fixes.
Some streams have error data introducing unknown NAL type. There are
also kinds of NAL types we do not want to handle. The old manner will
set a decoder error when encounter this, which cause a latent crash bug.
The decoder may successfully decode the picture and insert it into DPB.
But there are error NAL units after the AU which cause the post unit error
and make that frame dropped. The later output of the picture still want
to ref that frame and crash.
No need to set decoder error when can not recognize or handle the NAL
unit, just skip it and continue.
Fix: #191
In some cases, the found_picture ended up being evicted and freed, which
would lead to a use after free when accessing picture->base.poc. In this
fix, we take a ref on the picture before calling dpb_evict.
https://bugzilla.gnome.org/show_bug.cgi?id=787124
Similarly to previous patch, we have no error concealment. As a side
effect, it's better to skip slices with missing references then passing
NULL pointers to the accelerator. Passing NULL pointer would lead to
major visual artifact, a behaviour that is likely undefined.
https://bugzilla.gnome.org/show_bug.cgi?id=787124
Usually in case of MVC decoding, dpb size is increasedi if subset sps.
That's why it resets context without this patch.
But for some media it doesn't increase dpb size. Even in this case we
should reset context to deal with MVC decoding.
Otherwise, it leads to assert.
https://bugzilla.gnome.org/show_bug.cgi?id=787124
If the unit could not be parsed, just skip this nal and keep parsing
what is left in the adapter. We need to flush the broken unit in the
decoder specific parser because the generic code does not know about
units boundary. This increases error resilliance.
Before this, the broken unit would stay in the adapter and EOS would be
returned. Which stopped the streaming. Just removing the EOS would have
lead to the adapter size growing indefinitely.
https://bugzilla.gnome.org/show_bug.cgi?id=796863
h264 log messages were logged in default category because a regression
in code. This patch renable the usage of vaapi logging category.
This regression was introduced in commit 7c365bdd.
This patch chains up failure to executing the modification process. The
end result is that we now fail decoding the slice if this process fails.
This avoid sending a corrupted state to the accelerator. In some special
cases, this could lead to unrecoverable errors.
https://bugzilla.gnome.org/show_bug.cgi?id=796832
gst_adapter_masked_scan_uint32_peek() asserts if size is 0. Don't
try and scan in that case. This fixes assertion that would some times
happen when the stream is corrupted.
https://bugzilla.gnome.org/show_bug.cgi?id=796832
The decoder was trying to detect earlier that a field was lost base
on guessing the parity order. This breaks in streams were the parity
order changes.
This patch reverts the field order prediction code added by commit
8dd93e9c8.
https://bugzilla.gnome.org/show_bug.cgi?id=796169
This ensure that we always have sentinels set in the reference
pictures arrays. The code wasn't unsafe, this simply improve the
tracing, so instead of printing 32 lines of zeros, va tracer
prints proper empty lists.
https://bugzilla.gnome.org/show_bug.cgi?id=796169
Even if it is the h264parse fault or bad video file, vaapih264dec
should set a proper value for VAPictureParameterBufferH264.num_ref_frames
as the driver might use it.
Also see "info.ref_frames = dpb_size;" in
gstvaapidecoder_h264.c::ensure_context
https://bugzilla.gnome.org/show_bug.cgi?id=793836
Fixes regression introduced by commit 2eb2b26a.
There is a use case when the decoder set the src caps and immediatly
tries to process the media codec_data, this happens before decoder is
even opened, thus priv->parser is not instantiated yet.
https://bugzilla.gnome.org/show_bug.cgi?id=787818
Drops non-base NALs when the base-only property is set to TRUE.
This modifies the behavior for MVC streams with base-only too: All the
non-base units are dropped before they are decoded instead of dropping
the non-base frames.
The relevant part from the H264 spec is:
> Decoders that conform to one or more of the profiles specified in
Annex A rather than the profiles specified in Annexes G or H shall
ignore (remove from the bitstream and discard) the contents of all NAL
units with nal_unit_type equal to 14, 15, or 20.
To eliminate side effects from the offending units:
- PPS's with a broken seq_parameter_set_id (referring to dropped subset
SPS's) are ignored.
- The NAL parsing is skipped and their flags are set to
GST_VAAPI_DECODER_UNIT_FLAG_SKIP.
- Prefix units are not stored in prev_pi. Otherwise, parse_slice() would
use them even if they are flagged to be skipped. Subset SPS's and slice
extension units are not stored there either.
https://bugzilla.gnome.org/show_bug.cgi?id=732266
Signed-off-by: Sreerenj Balachandran <sreerenj.balachandran@intel.com>
Coverity scan bug:
Dereference after null check: Either the check against null is
unnecessary, or there may be a null pointer dereference.
In the original commit for fill_picture_gaps() (commit 5abd2b90) the
prev_picture could be NULL, that's why the code did a null check. But,
since commit 52adebe7, the previous reference frames are tracked, thus
there is no need to check null anymore.
Push frames downstream as soon as possible instead of waiting until
they are ejected from the DPB.
This patch makes the decoder not comply with the H.264 specification,
but it is required for some video cameras.
https://bugzilla.gnome.org/show_bug.cgi?id=762509
Signed-off-by: Víctor Manuel Jáquez Leal <vjaquez@igalia.com>
Since commits in https://bugzilla.gnome.org/show_bug.cgi?id=781142 landed,
they introduced regression in seek.
Formerly, once seek is done, decoder drops P-frames until I-frame arrives.
But since the commits landed, it doesn't drop P-frame and does try to
decode it continuously because active_sps is still alive. See ensure_sps function.
But there are prev_frames and prev_ref_frames reset already, then it
causes assertion.
So it's necessary to reset active_sps/pps also in reset method.
https://bugzilla.gnome.org/show_bug.cgi?id=783726
Clear decoders out on a flush but keep the same instance,
rather than completely recreating them. That avoids
unecessarily freeing and recreating surface pools
and contexts, which can be quite expensive
https://bugzilla.gnome.org/show_bug.cgi?id=781142
Configuring GCC to verify possible usage of uninitialized variables,
shows that found_index might be used without previous assignation.
This patch assigns a initial value to found_index, also avoid a
branching when returning the result value.
https://bugzilla.gnome.org/show_bug.cgi?id=778782
Reduce frame num gaps so that we don't have to create unnecessary
dummy pictures, just throw them away.
Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
https://bugzilla.gnome.org/show_bug.cgi?id=777506
If the frame is a cloned picture, its PTS comes from its parent
picture. In addition, the base decoder doesn't set a valid PTS to
the frame corresponding to the cloned picture.
https://bugzilla.gnome.org/show_bug.cgi?id=774254
When the format of a H.264 stream is AVC3, the SPS and PPS are inside the
stream, not in the codec_data, so the size of codec_data might be 7.
This patch reduces the minimal size of the codec_data buffer from 8 to 7.
https://bugzilla.gnome.org/show_bug.cgi?id=771441
As part of the upstreaming process of gstreamer-vaapi into the GStreamer
umbrella, we need to comply with the project's code style. This meant to
change a lot of code.
It was decided to use a single massive patch to update the code style.
I would like to apologize with the original developers of this code because of
the history breakage.
Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
As gstreamer-vaapi now only supports from GStreamer 1.6, this patch removes
all the old GStreamer version guards.
Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
When the GstVaapiParserInfoH264 is allocated, the memory is not initialized,
so it contains random data.
When gst_h264_parser_parse_pps() fails, the PPS structure keeps slice_group_id
pointer uninitialized, leading to a segmentation fault when the memory is
freed.
This patch prevents this by initializing the slice_group_id before the PPS
parsing.
Signed-off-by: Víctor Manuel Jáquez Leal <victorx.jaquez@intel.com>
https://bugzilla.gnome.org/show_bug.cgi?id=754845
Improve closure of gaps in frame_num by strictly following and trying
to fill them with previous reference frames. So, they are now tracked
thus avoiding insertion of dummy ("greenish") frames.
If the new picture to be added to the DPB is not a first field, then
it shall be the second field of the previous picture that was added
before.
This removes the need for dpb_find_picture() now that we track the
immediately preceding decoded picture, in decode order.
When a dummy "other-field" is inserted, it is assumed to inherit the
reference flags from the first field, and the sliding window decoded
reference picture marking process is also executed so that corrupted
frames are moved out as early as possible.
While doing so, we also try to output frames that now contain a single
valid field picture, prior to inserting any other picture into the DPB.
Note: this may be superfluous currently based on the fact that dpb_add()
combines the two most recent pairable fields, but this process would be
further simplified later on.
Mark the picture as "corrupted" if it is reconstructed from corrupted
references or if those references are fake, e.g. resulting from lost
frames.
This is useful for notifying the upper layer, or downstream elements,
that the decoded frame may contain artefacts.
https://bugzilla.gnome.org/show_bug.cgi?id=703921
Fix uninitialized variables when decoding SPS and PPS NAL units from
"codec-data" buffers. This is particularly important when seeking ops
are involved, and the new persistent states are used more often.
https://bugzilla.gnome.org/show_bug.cgi?id=750094
Implement decoding process for gaps in frame_num (8.5.2). This
also somewhat supports unintentional loss of pictures.
https://bugzilla.gnome.org/show_bug.cgi?id=745048https://bugzilla.gnome.org/show_bug.cgi?id=703921
Original-patch-by: Wind Yuan <feng.yuan@intel.com>
[fixed derivation of POC, ensured clone is valid for reference,
actually fixed detection of gaps in FrameNum by PrevRefFrameNum]
Signed-off-by: Gwenole Beauchesne <gwenole.beauchesne@intel.com>
Try to identify missing first fields too, thus disregarding any
intermediate gaps in frames. We also assume that we keep the same
field sequence, i.e. if previous frames were in top-field-first
(TFF) order, then so are subsequent frames.
Note that insertion of dummy first fields need to operate in two
steps: (i) create the original first field that the current field
will inherit from, and (ii) submit that field into the DPB prior
to initializing the current (other) field POC values but after any
reference flag was set. i.e. copy reference flags from the child
(other field) to the parent (first field).
https://bugzilla.gnome.org/show_bug.cgi?id=745048
Interlaced H.264 video frames always have two fields to decode and
display. However, in some cases, e.g. packet loss, one of the field
can be missing. This perturbs the reference picture marking process,
whereby the number of references available in DPB no longer matches
the expected value.
This patch adds initial support for missing field within a decoded
frame. The current strategy taken is to find out the nearest field,
by POC value, and with the same parity.
https://bugzilla.gnome.org/show_bug.cgi?id=745048
