Opened 13 years ago
Closed 13 years ago
#1351 closed defect (fixed)
mpeg2video lowres support
Reported by: | jyavenard | Owned by: | |
---|---|---|---|
Priority: | important | Component: | avcodec |
Version: | git-master | Keywords: | mpeg2video regression lowres |
Cc: | Blocked By: | ||
Blocking: | Reproduced by developer: | yes | |
Analyzed by developer: | no |
Description
In:
commit 2bcbd98459915baefc15043d02f4a942ebcd33da Author: Mans Rullgard <mans@mansr.com> Date: Thu Apr 12 13:55:49 2012 +0100 Remove lowres video decoding This feature is complex, of questionable utility, and slows down normal decoding. Signed-off-by: Mans Rullgard <mans@mansr.com>
low-res video decoding is used in mythtv in order to flag a video for commercial.
Removing this mode breaks mythtv's comm flagging.
Doing comm flagging without low res reduced speed by a factor of two.
So I would like to plead to reverse that change.
There was no need to remove it.
Thank you
Change History (13)
comment:1 by , 13 years ago
comment:2 by , 13 years ago
Not this the merge in 92ef4be4ab9fbb7d901b22e0036a4ca90b00a476
Do you really think I would have lodged this issue if that wasn't the case?
commit 92ef4be4ab9fbb7d901b22e0036a4ca90b00a476 Merge: 2e07f42 d526c53 Author: Michael Niedermayer <michaelni@gmx.at> Date: Sun Apr 22 22:26:42 2012 +0200 Merge remote-tracking branch 'qatar/master' * qatar/master: ARM: allow runtime masking of CPU features dsputil: remove unused functions mov: Treat keyframe indexes as 1-origin if starting at non-zero. mov: Take stps entries into consideration also about key_off. Remove lowres video decoding
which included the commit in the original report
comment:3 by , 13 years ago
Component: | undetermined → avcodec |
---|---|
Keywords: | mpeg2video regression added |
Priority: | normal → important |
Reproduced by developer: | set |
Status: | new → open |
Summary: | Removal of mpeg2 low-res video decoding: why? → mpeg2video lowres support |
Version: | unspecified → git-master |
The relevant commit is 5e50a57
comment:4 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | open → closed |
This appears to have been fixed, thank you for your report!
comment:5 by , 13 years ago
Keywords: | lowres added |
---|
follow-up: 7 comment:6 by , 13 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Thank you for this...
However, are you sure the revert is complete?
The git comment itself is wrong, and the sha mentioned isn't the correct one:
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=5e50a5724bb00c44a98bd89f57c659a613f26ce2
The merged commit (from Libav) that removed lowres was
2bcbd98459915baefc15043d02f4a942ebcd33da
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=2bcbd98459915baefc15043d02f4a942ebcd33da
it modified the files:
avconv.c avplay.c libavcodec/alpha/dsputil_alpha.c libavcodec/arm/dsputil_init_arm.c libavcodec/arm/dsputil_init_armv5te.c libavcodec/arm/dsputil_init_armv6.c libavcodec/arm/dsputil_init_neon.c libavcodec/avcodec.h libavcodec/dsputil.c libavcodec/dsputil.h libavcodec/dv.c libavcodec/dvdec.c libavcodec/error_resilience.c libavcodec/flvdec.c libavcodec/h261dec.c libavcodec/h263dec.c libavcodec/intrax8.c libavcodec/jrevdct.c libavcodec/libopenjpeg.c libavcodec/mjpegbdec.c libavcodec/mjpegdec.c libavcodec/mpeg12.c libavcodec/mpeg4videodec.c libavcodec/mpegvideo.c libavcodec/mpegvideo.h libavcodec/msmpeg4.c libavcodec/mxpegdec.c libavcodec/options_table.h libavcodec/ppc/dsputil_ppc.c libavcodec/rv10.c libavcodec/sp5xdec.c libavcodec/utils.c libavcodec/wmv2dec.c libavcodec/x86/dsputil_mmx.c
Also, a following commits removed DSP code that were used for lowres support:
d7458bc8c62ae1cb2ffc805b989fcddf4029dda6
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=d7458bc8c62ae1cb2ffc805b989fcddf4029dda6
So the reversal made is only partial and only going to have an effect on mpeg decoder...
BTW, I agree, there's no way four extra bit shifts would have caused a drop of speed of 3%... Those stats could have only been made up...
comment:7 by , 13 years ago
Replying to jyavenard:
However, are you sure the revert is complete?
No.
You reported that mpeg2video lowres support is missing (I originally read only the body of the ticket, where it only says "lowres was removed", I did not understand the problem because lowres did exist at least for mpeg4 and dvvideo at the time you reported the problem).
Since yesterday, mpeg2video lowres works fine here.
If you believe there still is an issue, please provide a failing command line together with complete, uncut output (you should have done this originally).
follow-ups: 9 10 comment:8 by , 13 years ago
Command line of what? this is a libavcodec issue and about features dropped from it.
yes, mpeg2video lowres support was re-enabled. But what about lowres support for all the other codecs that were removed in that same commit.
Those were not reverted, i have listed above all the codecs that suddenly saw their lowres support dropped.
comment:9 by , 13 years ago
Replying to jyavenard:
yes, mpeg2video lowres support was re-enabled. But what about lowres support for all the other codecs that were removed in that same commit.
If you add a failing command line, it would be easier to understand which codecs you are talking about, I could then close this ticket (that you opened about mpeg2video) and open a new one.
You are right that tickets against libavcodec can be opened, but long time experience tells us that tickets that are reproducible with ffmpeg are much faster fixed.
follow-up: 11 comment:10 by , 13 years ago
Note, the lowres removial was reverted in 2 commits, first was 70d54392f5015b9c6594fcae558f59f952501e3b.
Ive not double checked now but i belive that the 2 commits brought all parts back.
comment:11 by , 13 years ago
Replying to michael:
Ive not double checked now but i belive that the 2 commits brought all parts back.
I believe wmv2 is still missing, I don't know if this is more difficult / problematic because of IntraX8 / J-frames.
comment:12 by , 13 years ago
Thanks Michael.. I see what went on...
I was confused with the name of "lowres2" and skipped that commit
So it was just mpeg2 that didn't get revert properly at first...
Excellent news..
I can tell you for sure now that MythTV will sync with ffmpeg from now on.
comment:13 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Afaict, current FFmpeg does support lowres...