On 2/25/20, Anton Khirnov <an...@khirnov.net> wrote: > Quoting Michael Niedermayer (2020-02-24 20:15:43) >> On Mon, Feb 24, 2020 at 03:54:45PM +0100, Anton Khirnov wrote: >> > Quoting Carl Eugen Hoyos (2020-02-24 13:50:57) >> > > Am Mo., 24. Feb. 2020 um 13:40 Uhr schrieb Anton Khirnov >> > > <an...@khirnov.net>: >> > > > >> > > > It fundamentally depends on an API that has been deprecated for five >> > > > years, has seen no commits since that time and is of highly dubious >> > > > usefulness. >> > > >> > > Please explain how the removed functionality was replaced. >> > >> > It was not, for the reasons mentioned in the commit message. >> >> > In my view, >> > the fact that nobody fixed it in all that time proves that nobody cares >> > about this functionality and thus that there is no value in keeping it. >> >> your reasoning only works if there is a problem that requires a fix. >> >> Your reasoning here seems >> BIG problem in A && noone fixes it -> noone cares about A >> >> My view is >> whoever sees a problem in A (i do not really) should fix it. >> >> Maybe iam missing something and there is in fact a big problem in the >> code. But if that is the case iam not aware of the problem and thats >> why i did nothing for years "fixing" it. Its not that i do not care. >> >> So what is really the issue here ? >> if i build vf_qp.o i get >> ./libavutil/frame.h:719:1: note: 'av_frame_get_qp_table' has been >> explicitly marked deprecated here >> attribute_deprecated >> >> ./libavutil/frame.h:721:1: note: 'av_frame_set_qp_table' has been >> explicitly marked deprecated here >> attribute_deprecated > > Yes, I believe there is a problem, or more precisely a bunch of related > problems. Not really that big, but real ones nevertheless. > > One aspect of the problem is precisely the fact that this functionality > has been deprecated and nobody has addressed this deprecation in many > years. Paul was concerned about our reputation - I believe having so > many deprecation warnings during build is very bad for our reputation. > But more importantly, it is confusing the developers and users about > what they should use and what not. If you cared about keeping this code, > you should have undeprecated it. > > Two ather aspects of the problem are: > - this API is only really workable for MPEG1/2 and closely related > codecs like JPEG/H.263/MPEG4 ASP/RV > - it is undocumented, the data layout is not defined > If you really want to keep it, those two points should be addressed. > >> >> if i look at git history these where deprecated in >> commit 7df37dd319f2d9d3e1becd5d433884e3ccfa1ee2 >> Author: James Almer <jamr...@gmail.com> >> Date: Mon Oct 23 11:10:48 2017 -0300 >> >> avutil/frame: deprecate getters and setters for AVFrame fields >> >> The fields can be accessed directly, so these are not needed anymore. >> >> This says the field can be accessed directly, so certainly its not >> deprecated in favor of the side data API. >> >> and in fact av_frame_get_qp_table / av_frame_set_qp_table do use the >> side data API already so none of this makes sense really. >> And the whole argument about five years also isnt correct as >> october 2017 is not 5 years ago > > The accessors may have been deprecated in 2017, but the entire > "exporting QP tables" functionality was deprecated long before that. In > any case, it does not matter when exactly that was. > >> >> >> > >> > Furthermore, I believe this filter (and all the associated >> > "postprocessing" ones) are anachronistic relics of the DivX era. They >> > were in fashion around ~2005 (though I doubt they were actually >> > improving anything even then) but nobody with a clue has used them since >> > H.264 took over. >> >> well, for old videos (which still exist today) and i mean the stuff >> that used 8x8 dct based codecs mpeg1 to mpeg4, divx, msmpeg4, realvideo >> also jpeg and that use not very high bitrate. (very high bitrate of course >> doesnt have much to improve toward) >> >> There is a quite noticable quality improvment when using postprocessing >> with the right parameters both subjective and objective (PSNR IIRC) >> And at the rare but not noneexisting occurance where i do want to watch >> such a video i always use one of these filters. >> In realty that has often been the spp filter but thats probably not >> important. >> In general if you can see 8x8 blocks without the filter, these filters >> will make the video simply look better. >> >> if passing QP helps for the above usecase probably depends on how >> variable QP is in the video one wants to watch or if a single fixed >> hand tuned QP works well (it often does indeed) > > But that is precisely the question at hand. Is passing QP tables a > useful thing to have? > Also, do note that MPV removed those filters and according to its > developer nobody ever missed them or complained about their removal. > Furthermore, people in https://github.com/mpv-player/mpv/issues/2792 > suggest that other filters may do as good or better job.
lol, I must comment on this. You just mentioned single usecase that use qp tables. Original Github issue poster was not 100% happy with alternative results. You would need to also remove pp filter and entire libpostprocess otherwise because they are mostly using qp table to function properly. Good luck with that. > >> >> Another usecase for passing QP was lossless re-encoding. >> I do not know how common this has been used (iam not using it and its not >> my idea originally), this of course also requires a encoder which >> can accept motion vectors and MB types on input or intra only >> >> Yet another use case is maintaining the input encoders choices >> for quantization / quality when converting to another format. >> in principle one could even have one encoder provide quantization >> information to a second encoder >> >> -> encoder1 >> / v >> raw input QP >> \ v >> -> encoder2 >> >> why? i dont know, maybe for art or fun, duplicate some bad QP choices or >> good >> QP choices, or edit QP choices ina specific area. >> >> but i would not call the ability to pass the QP array around and >> to modify it useless. > > I would disagree about that. All those cases you described are > theoretical - "someone might want to do it this way". But so far there > doesn't seem to be anyone who actualy does that or wants to do that. > > Theoretical features that nobody actually uses are not useful - hence > they are useless. I would even say they are worse then useless, since > they > - clutter the API namespace, making it harder to find actually useful > things > - provide additional attack surface for potential security issues > - make maintenance and refactoring harder, preventing actually useful > changes > >> >> Also last but not least, if you think there really is an issue that >> MUST be fixed otherwise the code must be removed. Why not ask the >> people listed in authors & copyright to look into it ? >> Iam listed in the copyright it seems and unless i forgot it noone >> asked me to fix some major issue in vf_qp > > That is exactly what I am doing with this patch set. I do not have a > personal vendetta against this code. I do not intend to go over dead > bodies to see it removed. > > It is marked for removal and we are planning a major bump, hence I am > setting patches that remove it. It is an opportunity for people who want > to keep it to step up and do something about it. > > But so far all the objections except yours have been pure feature > hoarding. "Someone might conceivably use this so it must not be > removed". I do not believe this way of thinking is good for the project. > Either someone should show a clear valid use case for this, or it should > be dropped. > > And I am repeating yet again that the code remains in git history and > can always be resurrected in the future if someone wants it. It is not > gone forever. Adding new features is easier than removing them. > > -- > Anton Khirnov > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe". _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".