-
Notifications
You must be signed in to change notification settings - Fork 347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
enable use of system FFmpeg #428
Comments
Regarding libavcodec/get_bits.h (and golomb.h), we would need to write our own (or use someone else's) C++ version. Alternatively, we could use FFmpeg's parsers: FFmpeg: Frame parsing |
libavutil/internal.h appears to be for
|
references MythTV#428 libavutil/internal.h appears to be for ``` // This can be enabled to allow detection of additional integer overflows with ubsan //#define CHECKED ... // For debuging we use signed operations so overflows can be detected (by ubsan) // For production we use unsigned so there are no undefined operations ifdef CHECKED define SUINT int define SUINT32 int32_t else define SUINT unsigned define SUINT32 uint32_t endif ``` `SUINT` is conditionally used once in libavcodec/golomb.h in `get_ur_golomb_jpegls()`, which is itself not used in the MythTV code.
originally from MythTV@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV#428 If this causes an issue, a new ticket with logs and a sample should be openned.
originally from MythTV/mythtv@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV/mythtv#428 If this causes an issue, a new ticket with logs and a sample should be openned.
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: MythTV#428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. The new parameter is always false in MythTV for no functional change. Several uses are explicit about using the history in the comments prior to the call. However, FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end().
refs: MythTV#428 ff_read_frame_flush() is an internal FFmpeg function. However, in libavformat/utils.c: int avformat_flush(AVFormatContext *s) { ff_read_frame_flush(s); return 0; } avformat_flush() is exactly identical and is a public function, so use it instead. StreamHasRequiredParameters(): I disabled the audio check requiring the internal struct to be defined. This should have no effect since these checks are *all* already done via avformat_find_stream_info() in FindStreamInfo(), which should return < 0 if these checks fail. This copy of an internal FFmpeg function feels unnecessary and like a bad idea since FFmpeg already checks this itself.
originally from MythTV/mythtv@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV/mythtv#428 If this causes an issue, a new ticket with logs and a sample should be openned.
originally from MythTV/mythtv@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV/mythtv#428 If this causes an issue, a new ticket with logs and a sample should be openned.
references MythTV#428 libavutil/internal.h appears to be for ``` // This can be enabled to allow detection of additional integer overflows with ubsan //#define CHECKED ... // For debuging we use signed operations so overflows can be detected (by ubsan) // For production we use unsigned so there are no undefined operations ifdef CHECKED define SUINT int define SUINT32 int32_t else define SUINT unsigned define SUINT32 uint32_t endif ``` `SUINT` is conditionally used once in libavcodec/golomb.h in `get_ur_golomb_jpegls()`, which is itself not used in the MythTV code.
originally from MythTV@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV#428 If this causes an issue, a new ticket with logs and a sample should be openned.
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: MythTV#428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. The new parameter is always false in MythTV for no functional change. Several uses are explicit about using the history in the comments prior to the call. However, FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end().
refs: MythTV#428 ff_read_frame_flush() is an internal FFmpeg function. However, in libavformat/utils.c: int avformat_flush(AVFormatContext *s) { ff_read_frame_flush(s); return 0; } avformat_flush() is exactly identical and is a public function, so use it instead. StreamHasRequiredParameters(): The audio check requiring the internal struct to be defined was disabled. This should have no effect since these checks are *all* already done via avformat_find_stream_info() in FindStreamInfo(), which should return < 0 if these checks fail. This copy of an internal FFmpeg function feels unnecessary and like a bad idea since FFmpeg already checks this itself.
references MythTV#428 libavutil/internal.h appears to be for ``` // This can be enabled to allow detection of additional integer overflows with ubsan //#define CHECKED ... // For debuging we use signed operations so overflows can be detected (by ubsan) // For production we use unsigned so there are no undefined operations ifdef CHECKED define SUINT int define SUINT32 int32_t else define SUINT unsigned define SUINT32 uint32_t endif ``` `SUINT` is conditionally used once in libavcodec/golomb.h in `get_ur_golomb_jpegls()`, which is itself not used in the MythTV code.
originally from MythTV@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV#428 If this causes an issue, a new ticket with logs and a sample should be openned.
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: MythTV#428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. The new parameter is always false in MythTV for no functional change. Several uses are explicit about using the history in the comments prior to the call. However, FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end().
refs: MythTV#428 ff_read_frame_flush() is an internal FFmpeg function. However, in libavformat/utils.c: int avformat_flush(AVFormatContext *s) { ff_read_frame_flush(s); return 0; } avformat_flush() is exactly identical and is a public function, so use it instead. StreamHasRequiredParameters(): The audio check requiring the internal struct to be defined was disabled. This should have no effect since these checks are *all* already done via avformat_find_stream_info() in FindStreamInfo(), which should return < 0 if these checks fail. This copy of an internal FFmpeg function feels unnecessary and like a bad idea since FFmpeg already checks this itself.
references MythTV#428 libavutil/internal.h appears to be for ``` // This can be enabled to allow detection of additional integer overflows with ubsan //#define CHECKED ... // For debuging we use signed operations so overflows can be detected (by ubsan) // For production we use unsigned so there are no undefined operations ifdef CHECKED define SUINT int define SUINT32 int32_t else define SUINT unsigned define SUINT32 uint32_t endif ``` `SUINT` is conditionally used once in libavcodec/golomb.h in `get_ur_golomb_jpegls()`, which is itself not used in the MythTV code.
originally from MythTV@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV#428 If this causes an issue, a new ticket with logs and a sample should be openned.
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: MythTV#428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. The new parameter is always false in MythTV for no functional change. Several uses are explicit about using the history in the comments prior to the call. However, FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end().
refs: MythTV#428 ff_read_frame_flush() is an internal FFmpeg function. However, in libavformat/utils.c: int avformat_flush(AVFormatContext *s) { ff_read_frame_flush(s); return 0; } avformat_flush() is exactly identical and is a public function, so use it instead. StreamHasRequiredParameters(): The audio check requiring the internal struct to be defined was disabled. This should have no effect since these checks are *all* already done via avformat_find_stream_info() in FindStreamInfo(), which should return < 0 if these checks fail. This copy of an internal FFmpeg function feels unnecessary and like a bad idea since FFmpeg already checks this itself.
references MythTV#428 libavutil/internal.h appears to be for ``` // This can be enabled to allow detection of additional integer overflows with ubsan //#define CHECKED ... // For debuging we use signed operations so overflows can be detected (by ubsan) // For production we use unsigned so there are no undefined operations ifdef CHECKED define SUINT int define SUINT32 int32_t else define SUINT unsigned define SUINT32 uint32_t endif ``` `SUINT` is conditionally used once in libavcodec/golomb.h in `get_ur_golomb_jpegls()`, which is itself not used in the MythTV code.
originally from MythTV@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV#428 If this causes an issue, a new ticket with logs and a sample should be openned.
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: MythTV#428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. The new parameter is always false in MythTV for no functional change. Several uses are explicit about using the history in the comments prior to the call. However, FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end().
refs: MythTV#428 ff_read_frame_flush() is an internal FFmpeg function. However, in libavformat/utils.c: int avformat_flush(AVFormatContext *s) { ff_read_frame_flush(s); return 0; } avformat_flush() is exactly identical and is a public function, so use it instead. StreamHasRequiredParameters(): The audio check requiring the internal struct to be defined was disabled. This should have no effect since these checks are *all* already done via avformat_find_stream_info() in FindStreamInfo(), which should return < 0 if these checks fail. This copy of an internal FFmpeg function feels unnecessary and like a bad idea since FFmpeg already checks this itself.
This should have no functional change. References MythTV#428
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: MythTV#428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end(). So, I have split the truncated start code logic into its own function as I have now done for FFmpeg, which only used this functionality once. Determining if it is necessary would require further analysis or adding logging to see if truncated start codes are detected.
This should have no functional change. References MythTV#428 The hack is no longer necessary since internal FFmpeg headers are no longer included by the parsers. avformatdecoder.cpp needed the public FFmpeg header "libavutil/intreadwrite.h".
originally from MythTV@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV#428 If this causes an issue, a new ticket with logs and a sample should be openned.
refs: #428 ff_read_frame_flush() is an internal FFmpeg function. However, in libavformat/utils.c: int avformat_flush(AVFormatContext *s) { ff_read_frame_flush(s); return 0; } avformat_flush() is exactly identical and is a public function, so use it instead. StreamHasRequiredParameters(): The audio check requiring the internal struct to be defined was disabled. This should have no effect since these checks are *all* already done via avformat_find_stream_info() in FindStreamInfo(), which should return < 0 if these checks fail. This copy of an internal FFmpeg function feels unnecessary and like a bad idea since FFmpeg already checks this itself.
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: #428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end(). So, I have split the truncated start code logic into its own function as I have now done for FFmpeg, which only used this functionality once. Determining if it is necessary would require further analysis or adding logging to see if truncated start codes are detected.
This should have no functional change. References #428 The hack is no longer necessary since internal FFmpeg headers are no longer included by the parsers. avformatdecoder.cpp needed the public FFmpeg header "libavutil/intreadwrite.h".
originally from 0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References #428 If this causes an issue, a new ticket with logs and a sample should be openned.
refs: #428 ff_read_frame_flush() is an internal FFmpeg function. However, in libavformat/utils.c: int avformat_flush(AVFormatContext *s) { ff_read_frame_flush(s); return 0; } avformat_flush() is exactly identical and is a public function, so use it instead. StreamHasRequiredParameters(): The audio check requiring the internal struct to be defined was disabled. This should have no effect since these checks are *all* already done via avformat_find_stream_info() in FindStreamInfo(), which should return < 0 if these checks fail. This copy of an internal FFmpeg function feels unnecessary and like a bad idea since FFmpeg already checks this itself.
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: #428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end(). So, I have split the truncated start code logic into its own function as I have now done for FFmpeg, which only used this functionality once. Determining if it is necessary would require further analysis or adding logging to see if truncated start codes are detected.
This should have no functional change. References #428 The hack is no longer necessary since internal FFmpeg headers are no longer included by the parsers. avformatdecoder.cpp needed the public FFmpeg header "libavutil/intreadwrite.h".
originally from 0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References #428 If this causes an issue, a new ticket with logs and a sample should be openned.
refs: MythTV#428 ff_read_frame_flush() is an internal FFmpeg function. However, in libavformat/utils.c: int avformat_flush(AVFormatContext *s) { ff_read_frame_flush(s); return 0; } avformat_flush() is exactly identical and is a public function, so use it instead. StreamHasRequiredParameters(): The audio check requiring the internal struct to be defined was disabled. This should have no effect since these checks are *all* already done via avformat_find_stream_info() in FindStreamInfo(), which should return < 0 if these checks fail. This copy of an internal FFmpeg function feels unnecessary and like a bad idea since FFmpeg already checks this itself.
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: MythTV#428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end(). So, I have split the truncated start code logic into its own function as I have now done for FFmpeg, which only used this functionality once. Determining if it is necessary would require further analysis or adding logging to see if truncated start codes are detected.
This should have no functional change. References MythTV#428 The hack is no longer necessary since internal FFmpeg headers are no longer included by the parsers. avformatdecoder.cpp needed the public FFmpeg header "libavutil/intreadwrite.h".
originally from MythTV@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV#428 If this causes an issue, a new ticket with logs and a sample should be openned.
refs: #428 ff_read_frame_flush() is an internal FFmpeg function. However, in libavformat/utils.c: int avformat_flush(AVFormatContext *s) { ff_read_frame_flush(s); return 0; } avformat_flush() is exactly identical and is a public function, so use it instead. StreamHasRequiredParameters(): The audio check requiring the internal struct to be defined was disabled. This should have no effect since these checks are *all* already done via avformat_find_stream_info() in FindStreamInfo(), which should return < 0 if these checks fail. This copy of an internal FFmpeg function feels unnecessary and like a bad idea since FFmpeg already checks this itself.
by copying avpriv_find_start_code() into MythTV as ByteReader::find_start_code(). Refs: #428 Note: I have rewritten this function. The only functional change is 00 00 01 00 01 XX no longer incorrectly returns a start code at offset 7 that overlaps the start code at offset 4 if the start_code input is not modified between the two calls. FFmpeg only has one use that keeps the history, (ff_)mpeg1_find_frame_end(). So, I have split the truncated start code logic into its own function as I have now done for FFmpeg, which only used this functionality once. Determining if it is necessary would require further analysis or adding logging to see if truncated start codes are detected.
This should have no functional change. References #428 The hack is no longer necessary since internal FFmpeg headers are no longer included by the parsers. avformatdecoder.cpp needed the public FFmpeg header "libavutil/intreadwrite.h".
originally from 0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References #428 If this causes an issue, a new ticket with logs and a sample should be openned.
I realize this isn't the most appropriate place to bring this up but...there seems to be a regression that was introduced into FFmpeg on the version that's incorporated into the fixes/32 branch. There are blocking artifacts with MPEG-TS demuxing when using nvdec or VDPAU for decoding (and I've seen evidence that va-api also occasionally sees this issue). It's particularly noticeable on interlaced content for some reason. For a time, mpv also seemed to have this issue, which as far as I know leverages my system installed FFmpeg. Some time recently I updated FFmpeg to 4.4.2 and those artifacts disappeared. Whether that was working around latent ffmpeg behavior on their part (I searched their repo, couldn't find any meaningful messages) or FFmpeg fixed a bug, I'm not sure. I will say the closest thing I've found to evidence and a workaround for this in the mythtv repo was the H264 blocking artifact fix here: c257bacbfe All of this is to say, I'm hoping a release is turned around with these fixes soon - it'd be great to see if this regression is fixed. |
@KungFuJesus You should have opened a new bug report. Does your blocking look like https://trac.ffmpeg.org/attachment/ticket/9532/tweevoortwaalf_ffmpeg.png ? Because that is what the commit you mentioned fixed. That blocking occurred at the start and (sometimes) on skipping. |
Yes, though blocking artifacts I'm sure is a broad symptom. It seems most common when there's a moment of fast transition where the compression has to make more out of the bitrate. It goes away when using software decoding or when the mpeg transport stream is removed with a lossless transcode. I'll get you a screenshot. |
I can send you the whole transport stream but it's pretty huge. I don't have a great place to readily host it. |
@bennettpeter Thoughts on this blocking? It sounds different to me from what Klaas found. If you think it worthwhile, I can try merging the upstream FFmpeg release/4.4 into ours again for fixes/32. @KungFuJesus You can chop an MPEG Transport Stream at any arbitrary place, e.g. with
So if I understand correctly, the blocking only occurs on .ts files and not after lossless cutting and remuxing to .mpg? What hardware are you using and is this replicable on another system? I would like you to attach a log from |
Yes from what I remember.
Yes, every system I've tested on using vdpau or nvdec decoding reproduces this. This particular system is fairly old hardware but I've produced this on Maxwell and pascal based hardware using both nvdec and vdpau.
Will do so when I'm home. I can also cut up a short duration TS if it's feasible to upload on GitHub. I saw one person around November of last year report a similar issue on the mythtv forums. I'll open up a new issue with all the details and sample file, log output, forum post, etc. |
It appears to be some bizarre abomination where the left and right audio channels of a stereo stream are in different languages. See for reference (Add support for ivtv-generated bilingual dual audio MPEG) https://code.mythtv.org/trac/ticket/2439 Record side of PVR-x50 SAP support patch from szermatt at… MythTV/mythtv@435540c Record side of PVR-x50 SAP support patch from szermatt at… MythTV/mythtv@435540c Applies UI portion of "dual audio" patch. MythTV/mythtv@8bb0d4b Fix reversed logic in test used for MPEG dual audio supp… MythTV/mythtv@60deced Set correct audio mode for ivtv drivers using V4L2 MPEG … MythTV/mythtv@28619ff Fix ivtv dual language mode regression from [14800] ffmpeg sync MythTV/mythtv@ea0c398 lavc/mpegaudio_parser.c: revert MythTV workaround originally from MythTV/mythtv@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV/mythtv#428 If this causes an issue, a new ticket with logs and a sample should be openned. (cherry picked from commit fdfdbd9)
It appears to be some bizarre abomination where the left and right audio channels of a stereo stream are in different languages. See for reference (Add support for ivtv-generated bilingual dual audio MPEG) https://code.mythtv.org/trac/ticket/2439 Record side of PVR-x50 SAP support patch from szermatt at… MythTV/mythtv@435540c Record side of PVR-x50 SAP support patch from szermatt at… MythTV/mythtv@435540c Applies UI portion of "dual audio" patch. MythTV/mythtv@8bb0d4b Fix reversed logic in test used for MPEG dual audio supp… MythTV/mythtv@60deced Set correct audio mode for ivtv drivers using V4L2 MPEG … MythTV/mythtv@28619ff Fix ivtv dual language mode regression from [14800] ffmpeg sync MythTV/mythtv@ea0c398 lavc/mpegaudio_parser.c: revert MythTV workaround originally from MythTV/mythtv@0bc1604 referencing https://code.mythtv.org/trac/ticket/8260 References MythTV/mythtv#428 If this causes an issue, a new ticket with logs and a sample should be openned. (cherry picked from commit fdfdbd9)
In order to use the system supplied FFmpeg, we need to:
1. eliminate/upstream our modifications
Investigate subtitle changes/additions:
MPEG-TS changes
2. Only use the following FFmpeg headers (eliminate use of internal FFmpeg headers)
Headers installed by the -dev packages on Ubuntu:
libavcodec/:
ac3_parser.h
adts_parser.h
avcodec.h
avdct.h
avfft.h
bsf.h
codec_desc.h
codec.h
codec_id.h
codec_par.h
d3d11va.h
dirac.h
dv_profile.h
dxva2.h
jni.h
mediacodec.h
packet.h
qsv.h
vaapi.h
vdpau.h
version.h
videotoolbox.h
vorbis_parser.h
xvmc.h
libavdevice/:
avdevice.h
version.h
libavfilter/:
avfilter.h
buffersink.h
buffersrc.h
version.h
libavformat/:
avformat.h
avio.h
version.h
libavutil/:
adler32.h
aes_ctr.h
aes.h
attributes.h
audio_fifo.h
avassert.h
avconfig.h
avstring.h
avutil.h
base64.h
blowfish.h
bprint.h
bswap.h
buffer.h
camellia.h
cast5.h
channel_layout.h
common.h
cpu.h
crc.h
des.h
dict.h
display.h
dovi_meta.h
downmix_info.h
encryption_info.h
error.h
eval.h
ffversion.h
fifo.h
file.h
film_grain_params.h
frame.h
hash.h
hdr_dynamic_metadata.h
hmac.h
hwcontext_cuda.h
hwcontext_d3d11va.h
hwcontext_drm.h
hwcontext_dxva2.h
hwcontext.h
hwcontext_mediacodec.h
hwcontext_opencl.h
hwcontext_qsv.h
hwcontext_vaapi.h
hwcontext_vdpau.h
hwcontext_videotoolbox.h
hwcontext_vulkan.h
imgutils.h
intfloat.h
intreadwrite.h
lfg.h
log.h
lzo.h
macros.h
mastering_display_metadata.h
mathematics.h
md5.h
mem.h
motion_vector.h
murmur3.h
opt.h
parseutils.h
pixdesc.h
pixelutils.h
pixfmt.h
random_seed.h
rational.h
rc4.h
replaygain.h
ripemd.h
samplefmt.h
sha512.h
sha.h
spherical.h
stereo3d.h
tea.h
threadmessage.h
timecode.h
time.h
timestamp.h
tree.h
twofish.h
tx.h
version.h
video_enc_params.h
xtea.h
libpostproc/:
postprocess.h
version.h
libswresample/:
swresample.h
version.h
libswscale/:
swscale.h
version.h
Used internal headers
libavformat/
libavcodec
#ifdef
ed out and the file referenced is created in an out of tree patch to FFmpeglibavutil/
internal.h see ffmpeg cleanup #416
mythtv/libs/libmythtv/mythdeinterlacer.cpp: see replace configure's ARCH... with Q_PROCESSOR_... v2 #487
The text was updated successfully, but these errors were encountered: