Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed issues when adding new media and deinitializing media #3821

Merged
merged 3 commits into from
Jan 12, 2024
Merged

Conversation

sauwming
Copy link
Member

@sauwming sauwming commented Jan 4, 2024

There are several issues when adding new media, which need to be fixed:

  • Default new added media is initialised as inactive.
    This is because we currently don't set media direction for media reinitialization.
    if (!reinit) {
            /* Initialize default initial media direction as bidirectional. */
            call_med->def_dir = PJMEDIA_DIR_ENCODING_DECODING;
    }
  • Currently, the IP version detection is only done from initial offer.
static int get_media_ip_version(pjsua_call_media *call_med)
{
    pjmedia_sdp_session *rem_sdp = call_med->call->async_call.rem_sdp;
  • Set the default direction to the specified param->dir when adding video using pjsua_call_set_vid_strm(call_id, PJSUA_CALL_VID_STRM_ADD, param).

There's also another issue with double media deinitialization:

==60122==ERROR: AddressSanitizer: heap-use-after-free on address 0x00010ea8e938 at pc 0x0001064b4ee0 bp 0x00016b9eca30 sp 0x00016b9ec1f0
READ of size 2048 at 0x00010ea8e938 thread T4
    #1 0x104a66804 in pj_memcpy string.h:854
    #2 0x104a77d54 in pjmedia_stream_get_info stream.c:3250
    #3 0x104784d94 in pjsua_aud_stop_stream pjsua_aud.c:563
    #4 0x1047571a0 in stop_media_stream pjsua_media.c:3122
    #5 0x104750924 in stop_media_session pjsua_media.c:3179
    #6 0x10474f664 in pjsua_media_channel_deinit pjsua_media.c:3262
    #7 0x10471dc20 in pjsua_destroy2 pjsua_core.c:1939

0x00010ea8e938 is located 56 bytes inside of 8000-byte region [0x00010ea8e900,0x00010ea90840)
freed by thread T4 here:
    #5 0x104ea9228 in pj_pool_release pool_i.h:102
    #6 0x104ea92ec in pj_pool_safe_release pool_i.h:111
    #7 0x104a751b8 in pjmedia_stream_destroy stream.c:3170
    #8 0x104785564 in pjsua_aud_stop_stream pjsua_aud.c:612
    #9 0x1047571a0 in stop_media_stream pjsua_media.c:3122
    #10 0x104750924 in stop_media_session pjsua_media.c:3179
    #11 0x10474f664 in pjsua_media_channel_deinit pjsua_media.c:3262
    #12 0x1046ed130 in pjsua_call_hangup pjsua_call.c:3123
    #13 0x1047039c4 in pjsua_call_hangup_all pjsua_call.c:4002
    #14 0x10471db9c in pjsua_destroy2 pjsua_core.c:1931

pjsua_call_hangup_all() and pjsua_media_channel_deinit() will deinitialise the media twice.

@sauwming sauwming merged commit f2055ef into master Jan 12, 2024
35 checks passed
@sauwming sauwming deleted the add-media branch January 12, 2024 07:26
dshamaev-intermedia pushed a commit to intermedia-net/pjproject that referenced this pull request Apr 4, 2024
dshamaev-intermedia pushed a commit to intermedia-net/pjproject that referenced this pull request Apr 4, 2024
dshamaev-intermedia added a commit to intermedia-net/pjproject that referenced this pull request Apr 5, 2024
* Add option to shutdown all transports on IP change (pjsip#3781)

* pjsua_handle_ip_change: Added missing null check for on_ip_changed_progress callback (pjsip#3830)

* Reset stored remote name in dialog (dlg->initial_dest) if transport is server. (pjsip#3783)

* Prevent immediate tsx termination upon transport error (pjsip#3805)

* Fixed issues when adding new media and deinitializing media (pjsip#3821)

* Potential issues when IPv6 is disabled (pjsip#3835)

* Improve IP address change IPv4 <-> IPv6 (pjsip#3910)

* Add some missing unlocks (pjsip#3893)

* add missing unlock (pjsip#3885)

* Retransmit 2xx response when transport is closed (pjsip#3828)

* Fixed account's route set update when modifying account (pjsip#3825)

---------

Co-authored-by: Nanang Izzuddin <[email protected]>
Co-authored-by: sauwming <[email protected]>
Co-authored-by: Santiago De la Cruz <[email protected]>
Co-authored-by: Andreas Wehrmann <[email protected]>
Co-authored-by: Riza Sulistyo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants