Skip to content

[mod_av] changes to solve some video conference recording issues#591

Open
blackhorse-reddog wants to merge 4 commits intosignalwire:masterfrom
blackhorse-reddog:master
Open

[mod_av] changes to solve some video conference recording issues#591
blackhorse-reddog wants to merge 4 commits intosignalwire:masterfrom
blackhorse-reddog:master

Conversation

@blackhorse-reddog
Copy link

s->url should be created by av_strdup instead of strdup, because ffmpeg's util.c uses av_freep(&s->url); to free it. Currently it can cause a crash.

If codec_id is AV_CODEC_IP_MPEG4 (which is choosen by ffmpeg if the file extension is mp4), timebase denominator should be 48000 instead of 90000: "timebase 1/90000 not supported by MPEG 4 standard, the maximum admitted value for the timebase denominator is 65535"

Modifications in av_format.c:

s->url should be created by av_strdup instead of strdup, because ffmpeg's util.c uses av_freep(&s->url); to free it. Currently it can cause a crash.

If codec_id is AV_CODEC_IP_MPEG4 (which is choosen by ffmpeg if the file extension is mp4), timebase denominator should be 48000 instead of 90000: "timebase 1/90000 not supported by MPEG 4 standard, the maximum admitted value for the timebase denominator is 65535"
@andywolk andywolk requested review from mjerris and seven1240 April 28, 2020 12:28
…er to prevent the floor state from being stolen.
@seven1240
Copy link
Collaborator

av part LGTM. not sure the recorder of conference_video_set_floor_holder

@blackhorse-reddog
Copy link
Author

"not sure the recorder of conference_video_set_floor_holder"
If a member arrives with the MFLAG_JOIN_VID_FLOOR (typically used for screen share), the video floor should be locked by this member. This is usually the case, but sometimes another member acquires it after the new member with the mentioned flag takes it (by conference_video_set_floor_holder(conference, member, SWITCH_TRUE);), but before it is locked (by conference_utils_set_flag(member->conference, CFLAG_VID_FLOOR_LOCK);). This modification - changing these two lines - solved the problem, but I didn’t dig deep (for example, I don’t know where the function call which has obtained the floor status comes from), so I don’t know if there’s a better solution. I'll make a call stack to check it (may be just tomorrow).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants