mutagen icon indicating copy to clipboard operation
mutagen copied to clipboard

Write out CHAP frames in start_time order, ref #506

Open ping opened this issue 4 years ago • 6 comments

This is my attempt at a fix for #506 to write out CHAP frames in start_time order (due to issues with ffmpeg converting mp3 to m4b).

Tried to keep the changes to a minimal but it may look a little hacky.

ping avatar Sep 25 '21 11:09 ping

Alternatively, we can maintain the sorting logic inside the sort_key function.

        def sort_key(items):
            frame, data = items
            frame_key = frame.HashKey
            frame_size = len(data)

            # Let's ensure chapters are always sorted by their 'start_time'
            # and not by size/element_id pair.
            if frame.FrameID == "CHAP":
                frame_key = frame.FrameID
                frame_size = frame.start_time

            return (get_prio(frame), frame_key, frame_size)

ikalnytskyi avatar Nov 29 '22 10:11 ikalnytskyi

Hi, is there anything I can change to get this PR moving?

I'll be happy to incorporate the changes suggested (sort in sort_key).

ping avatar Apr 16 '23 04:04 ping

@ping Thanks for getting back to this. Yes, I think implementing this sorting as part of sort_key would be the preferred solution. If that is done the change could be merged.

phw avatar Apr 18 '23 06:04 phw

@phw I think there is a typo with the original suggestion. The return order should be priority, size, key.

return (get_prio(frame), frame_key, frame_size)

should be

return (get_prio(frame), frame_size, frame_key)

The PR has been updated with the changes.

ping avatar Apr 18 '23 07:04 ping

@phw @lazka

I giving this a bump to see if this can be merged before the next release. The only failed test in CI seems to be due to timeout.

ping avatar May 29 '23 03:05 ping