[FFmpeg-devel] [PATCH] Fix usage of temp_file flag in hls_flags option - 2nd attempt

[Aleksey Skripka]

Thanks for your attention, Adrian!

just one moment,
in hls_write_packet():

instead:
+        if (use_temp_file) {
+            hls_rename_temp_file(s, oc);

should be:
+        if (use_temp_file && !(hls->flags & HLS_SINGLE_FILE))
+            hls_rename_temp_file(s, oc);

or we a killing 'single' filename in few iterations.



Steven,
i recall my version of patch. Adrian's version fixes more issues and hope v3 will be the best :)


Aleksey Skripka

> On 18 Dec 2018, at 01:30, Adrian <adrian-007 at o2.pl> wrote:
> 
> Thanks for the explanation, now your intent is clear. I agree that keeping playlist file always as temp as long as it's a file protocol makes sense and won't do harm, though I'm still curious why original change to accept HLS_TEMP_FILE flag was sent to include and was included in official release, I believe there must be a reason.
> 
> Given that, I've prepared another patch with fix for original problem + proper file picked up for computing 'use_temp_file' variable value. I don't know what is the exact flow here, so I'll send it as another patch, previous can be discarded and hopefully this one won't raise any more objections, though I'd still appreciate a review.
> 
> Thanks for your input Aleksey!
> 
> Regards
> Adrian Guzowski
> 
> W dniu 17.12.2018 o 22:35, Aleksey Skripka pisze:
>> 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!
> <0001-Fix-usage-of-temp_file-flag-in-hls_flags-option-2nd-attempt.patch>_______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel