Quoting Alexander Strasser (2021-05-10 15:35:02) > On 2021-05-10 10:22 +0200, Anton Khirnov wrote: > > Export them in UTC, not the local timezone. This way the output is > > the same everywhere. The timezone information stored in the file is > > still ignored, since there seems to be no simple way to export it > > correctly. > > > > Format them according to ISO 8601, which we generally use for exporting > > dates. > > --- > > If anyone has practical suggestions for exporting the timezone, I would > > love to hear them. Don't see anything in the standard library for > > "express this UTC timestamp in a given timezone". Just adding the offset > > to the timestamp would AFAIU not be correct wrt leap seconds (and maybe > > something else? dates are hard). > > Surprisingly it seems best to ignore the timezone part on purpose! > > In 2.13 of Adobe's AMF 0 spec > > https://www.adobe.com/content/dam/acom/en/devnet/pdf/amf0-file-format-specification.pdf > > it is stated that the timezone part should not be filled, nor > used. > > So the commit message part about it and the comment in the > added by this patch should probably be removed. > > Though independent of this patch, one could maybe add a > check that states sample welcome if a file with a timezone > offset is found.
I have no idea how FLV is related to AMF0, but the sample used in the test in question here does have a non-zero value of the tz offset. So the comment seems appropriate to me. > > > > Also, the conversion of double to time_t is potentially UB if the source > > value is out of range, but there seems to be no standard way to check, > > since the range of time_t is not standard either. Anyone got any ideas? > > Not really. One could use sizeof to obtain the size, but for > that to be useful one would need to assume signedness. The > C standard doesn't seem to be explicit about that. And if > it did, we still don't know if the functions handle negative > offsets correctly. > > Seems that can't really by handled well locally here only > Maybe we do already or could add a check in configure > to find out about the bounds of time_t . Could be useful > elsewhere in the codebase too. Maybe other have better > ideas... > > > Otherwise your patch looks good to me and fine without > additional bound checks as it's not a new problem here. > > It could cause problems for people handling the timestamp > in the currently exported format though. Maybe at least a > micro bump should be in the final commit? We are in an ABI instability period right now, so this should not be necessary. -- 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".