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 > On 17 Dec 2018, at 19:12, Adrian <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>> 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> > 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
