> 在 2018年12月20日，上午12:19，Aleksey Skripka <ca...@undev.ru> 写道： > > All my tests ok. > Thank you, Adrian. > >> On 19 Dec 2018, at 12:41, Adrian <adrian-...@o2.pl> wrote: >> >> Okay, so given you two have some valid points, I've combined both and ended >> up with final version of the patch: >> - it uses temp file for playlist when either a flag is specified or playlist >> type is anything other than VOD >> - it does not use temp file for HLS_SINGLE_FILE in place mentioned by Aleksey >> >> This is my final submission, I believe now everyone should be happy with the >> solution. >> >> Regards >> Adrian Guzowski >> >> W dniu 18.12.2018 o 04:24, Ronak pisze: >>>> On Dec 17, 2018, at 4:35 PM, Aleksey Skripka <ca...@undev.ru> wrote: >>>> >>>> Evening! >>>> >>>> First of all, about playlist writeout: >>>> before 223d2bde22ce33dcbcb6f17f234b609cb98f1fb6 - playlist was always(!) >>>> creating via .tmp file. >>>> after 223d2bde22ce33dcbcb6f17f234b609cb98f1fb6 - playlists .tmp logic >>>> become dependent on +temp_file flag. >>>> >>>> I suggest to return to original logic. >>>> Non-atomic playlist writeout - is a disaster to just any hls player. This >>>> way we will force everyone (except offline VOD) to start using +temp_file. >>>> Always! >>>> Also, if somebody relying on this - they will get broken streams just >>>> after upgrade (It seems to me, that it can be a lot). >>>> >>>> So, here maintainer should decide how to be. >>>> >>>> >>>> and about single_file: >>>> if it will be decided to stay with current way (playlist's .tmp logic >>>> controlled by +temp_file flag), you are right - temp_file+single_file case >>>> become unreal. >>>> if it will be decided to return original behaviour (playlist always via >>>> .tmp), someone can choose how to writeout media file (despite i understand >>>> temp+single is very-very rare case). >>>> >>>> Thanks! >>>> -- >>>> Aleksey Skripk >>>> >>> Hey Aleksey, >>> >>> I'm not sure the best thing to do here is to switch to the original logic. >>> The temp file is only necessary if you are doing LIVE or EVENT streams; it >>> has nothing to do with VOD; where streams are pre-generated. I don't see >>> why you would ever want tmp files there. >>> Before this commit; ffmpeg used to take more than a week to fragment a 2.2 >>> GB Audio File in VOD mode. With this change; it takes less than 1 minute. >>> Other competing tools take less than 1 minute. >>> >>> Also, if you want temp files, you should be supplying the temp_file option >>> - the documentation clearly says that: >>> https://www.ffmpeg.org/ffmpeg-all.html#hls-2. If you do not want temp >>> files; then you should leave this option out. If you are forcing folks to >>> always accept the temp file, then why have an option? You should delete the >>> option and do it based on the playlist type. >>> >>> Ronak >>> >>>>> On 17 Dec 2018, at 19:12, Adrian <adrian-...@o2.pl >>>>> <mailto:adrian-...@o2.pl>> wrote: >>>>> >>>>> Comments inline. >>>>> >>>>> Regards >>>>> Adrian Guzowski >>>>> >>>>> W dniu 17.12.2018 o 16:56, Aleksey Skripka pisze: >>>>>>> On 17 Dec 2018, at 18:45, Adrian <adrian-...@o2.pl >>>>>>> <mailto:adrian-...@o2.pl> <mailto:adrian-...@o2.pl >>>>>>> <mailto:adrian-...@o2.pl>>> wrote: >>>>>>> >>>>>>> IMO temp_file+single_file should be mutually exclusive - why would you >>>>>>> want to have temp file when you're using only one file anyway? In this >>>>>>> mode you would either pool for file changes or just get updated >>>>>>> playlist with new byte range (and that one should be updated after >>>>>>> video file). I believe this restriction was already in place anyway, >>>>>>> because there already were checks for HLS_SINGLE_FILE not being set, >>>>>>> before doing actual rename, so this was just a simplification of those >>>>>>> conditions. >>>>>> not me to decide, please ;) >>>>>> if playlist will always be done via .tmp, we will have a choice. >>>>> Could you clarify what choice you're talking about? And the purpose of >>>>> this choice - frankly you got me lost with this. Regardless, I feel we're >>>>> getting off topic. >>>>>>> As for playlist file - I'm not sure I follow, I've changed only places >>>>>>> where temp files were already used, so I don't think I broke something >>>>>>> in that matter. >>>>>> not you, it was introduced by 223d2bde22ce33dcbcb6f17f234b609cb98f1fb6 >>>>> Not quite, temp files were already a feature, this commit narrowed >>>>> conditions where they are used. If you're referring to change in >>>>> hls_window about not checking TEMP flag - that is unrelated to current >>>>> state (before commit introducing regression, check was ignoring TEMP >>>>> flag, after commit it does take it into account - probably should be >>>>> discussed with person responsible for original change as to why it was >>>>> changed that way, so that's out of scope of this patch. Issue at hand is >>>>> not using TEMP files at all and that is fixed by this patch. >>>>> >>>>>>> From repeat+level+trace I believe I saw that playlist file actually was >>>>>>> using a temp file, so if that's the thing you are concerned about, it >>>>>>> seems like it's all good. >>>>>>> >>>>>>> Regards >>>>>>> Adrian Guzowski >>>>>>> >>>>>>> W dniu 17.12.2018 o 16:36, Aleksey Skripka pisze: >>>>>>>> Greetings! >>>>>>>> >>>>>>>> Adrian, >>>>>>>> with your variant of patch in case of '-hls_flags >>>>>>>> +temp_file+single_file' no .tmp logic will be used. >>>>>>>> not sure what is better, but all previous versions was doing so. >>>>>>>> >>>>>>>> and also, i'm still sure, that for backward compatibility with all >>>>>>>> previous versions, index.m3u8 should always be written via .tmp file. >>>>>>>> >>>>>>>> ps: >>>>>>>> i noticed this bug such way: i do not use +temp_file, but rely on >>>>>>>> atomic rename of index.m3u8. >>>>>>>> >>>>>>>>> On 17 Dec 2018, at 17:23, Adrian <adrian-...@o2.pl> wrote: >>>>>>>>> >>>>>>>>> Hello, >>>>>>>>> >>>>>>>>> after upgrading FFmpeg from 4.0 to 4.1 I noticed that temp files in >>>>>>>>> HLS muxed stopped working. >>>>>>>>> It looks like a regression introduced by >>>>>>>>> 223d2bde22ce33dcbcb6f17f234b609cb98f1fb6. I've prepared a patch and >>>>>>>>> tested it cross-compiling for my project's target platform (ARM, >>>>>>>>> Buildroot) and it seems like everything is in order. I ran regression >>>>>>>>> tests and nothing seems to be broken. >>>>>>>>> >>>>>>>>> Please review it and possibly include this patch. >>>>>>>>> >>>>>>>>> Regards >>>>>>>>> Adrian Guzowski >>>>>>>>> >>>>>>>>> <0001-Fix-usage-of-temp_file-flag-in-hls_flags-option.patch>_______________________________________________ >>>>>>>>> ffmpeg-devel mailing list >>>>>>>>> ffmpeg-devel@ffmpeg.org >>>>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>>>>> _______________________________________________ >>>>>>> ffmpeg-devel mailing list >>>>>>> ffmpeg-devel@ffmpeg.org >>>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>>>> _______________________________________________ >>>>>> ffmpeg-devel mailing list >>>>>> ffmpeg-devel@ffmpeg.org >>>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>>> _______________________________________________ >>>>> ffmpeg-devel mailing list >>>>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> >>>>> <mailto:ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org>> >>>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>>> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> >>>>> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>>> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel>> >>>> _______________________________________________ >>>> ffmpeg-devel mailing list >>>> ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> >>>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >>>> <http://ffmpeg.org/mailman/listinfo/ffmpeg-devel> >>> _______________________________________________ >>> ffmpeg-devel mailing list >>> ffmpeg-devel@ffmpeg.org >>> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> <0001-Fix-usage-of-temp_file-flag-in-hls_flags-option-v3.patch>_______________________________________________
Pushed Thanks Steven >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
