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

Audio and video stream refactoring #4300

Merged
merged 5 commits into from
Feb 13, 2025
Merged

Audio and video stream refactoring #4300

merged 5 commits into from
Feb 13, 2025

Conversation

sauwming
Copy link
Member

In order to support additional media, it would be better to refactor the current implementation of audio and video stream since they contains a lot of duplications, which make code maintenance tedious and more prone to bug (such as if you only fix one stream and do not apply to others).

Scope:

  • a new structure pjmedia_stream_common is created as a parent of pjmedia_stream and pjmedia_vid_stream
  • a new structure pjmedia_stream_info_common which contains members that exist in bothpjmedia_stream_info and pjmedia_vid_stream_info. Note that unlike the streams (pjmedia_stream and pjmedia_vid_stream) which are opaque, the stream info definition is public, so we cannot modify its internal structure.
  • common callbacks for RTP and RTCP handling, on_rx_rtp() and on_rx_rtcp(), which will call its specific stream implementation: on_stream_rx_rtp() (currently no specific stream's RTCP handling).
  • common group lock handler on_destroy() which will call its specific stream's handler on_stream_destroy().

Notes:

@sauwming sauwming marked this pull request as ready for review February 11, 2025 06:45
@wosrediinanatour
Copy link
Contributor

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is already a vid_stream.h, I would rename this file to aud_stream.h.
Same for C files: audio_stream.c, audio_stream_info.c

Or even better: audio_stream.h and video_stream.h. Or are there issues with file-name length on any platform?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this will break app that uses pjmedia directly. App that includes <pjmedia/stream.h> will fail to build. I can make stream.h to include aud_stream.h to avoid the build failure, but then the name of the structs and APIs is still pjmedia_stream instead of pjmedia_aud_stream, so I guess we're stuck with the old naming.

@sauwming
Copy link
Member Author

Cool thing! Has https://github.com/pjsip/pjproject/tree/master/pjsip-apps/src/3rdparty_media_sample been tested?

This PR only modifies pjmedia, and it's done without changing any APIs nor how to use the existing streams (audio and video). So pjsua and application won't be affected.

@sauwming
Copy link
Member Author

The latest commit is to fix failed CI tests.

This is caused by the new API pjmedia_stream_info_common_from_sdp() that can return PJ_SUCCESS, indicating that the SDP is actually valid and as per the specification, that the stream info is initialized, but there's no further processing required. An example of such case is that when the media port is zero.

So, I have to modify the new API to add one more parameter as an output param.

@sauwming sauwming merged commit 10b4d30 into master Feb 13, 2025
42 checks passed
@sauwming sauwming deleted the strm-refactor branch February 13, 2025 01:06
trengginas added a commit that referenced this pull request Feb 13, 2025
commit 1e2f121
Author: sauwming <ming@teluu.com>
Date:   Thu Feb 13 10:30:58 2025 +0800

    Fixed CI Mac failure (#4304)

commit 10b4d30
Author: sauwming <ming@teluu.com>
Date:   Thu Feb 13 09:06:54 2025 +0800

    Audio and video stream refactoring (#4300)

commit 6cbf0e6
Author: Riza Sulistyo <trengginas@users.noreply.github.com>
Date:   Wed Feb 12 15:23:37 2025 +0700

    Check if ice strans is valid before using it to send (#4301)

commit fdd4041
Author: Maciej Lisowski <39798354+MaciejDromin@users.noreply.github.com>
Date:   Tue Feb 11 12:48:17 2025 +0100

    Add missing OnTimerParam import in Android Example (#4299)

commit 4ded10f
Author: sauwming <ming@teluu.com>
Date:   Fri Feb 7 13:27:00 2025 +0800

    Fixed msg_data assertion in pjsua_acc_send_request() API (#4298)

commit 65c4bc9
Author: sauwming <ming@teluu.com>
Date:   Fri Feb 7 07:18:22 2025 +0800

    Fix pjsua sample app user agent (#4296)

commit c53ace9
Author: sauwming <ming@teluu.com>
Date:   Fri Feb 7 07:18:09 2025 +0800

    Fixed Java make clean error (#4297)

commit e6196ad
Author: LeonidGoltsblat <138720759+LeonidGoltsblat@users.noreply.github.com>
Date:   Fri Feb 7 02:15:10 2025 +0300

    Aligned memory allocation (#4277)

    * aligned memory allocaion

    * Fix alt API implementations (PJ_HAS_POOL_ALT_API)

    * pool test: add testing for bug in pj_pool_allocate_find with big alignment, and refactor to use unit test API

    * misc fixes on code review

    * pool_dbg alignment support + incompatible tests disabled for PJ_HAS_POOL_ALT_API

    ---------

    Co-authored-by: bennylp <bennylp@pjsip.org>

commit 2fff775
Author: sauwming <ming@teluu.com>
Date:   Thu Feb 6 13:11:01 2025 +0800

    Add API to register custom SDP comparison callback (#4286)

commit 99b4d1e
Author: sauwming <ming@teluu.com>
Date:   Thu Feb 6 13:10:38 2025 +0800

    Fixed issue with SDP version when reoffer is rejected (#4289)

commit 0252152
Author: Benny Prijono <bennylp@pjsip.org>
Date:   Thu Feb 6 11:09:20 2025 +0700

    Use cirunner to capture and analyze GitHub action CI crash (#4288)

    * Windows runner implementation

    * Set timeout

    * Remove initial implementation of ci-runner here (it is on separate repo now)

    * Remove crash handling (-n) in main.c of unit tests

    * Install cirunner to CI workflows

    * Adding crash to timestamp test

    * Fix missing cirunner in one of the job

    * Reinstall core_pattern on Linux

    * Removed intentional crash in timestamp_test()

    * Upload program and core dump on crash

    * Add crash code in uri_test.c

    * Removed injected crash in uri_test. Disable stdout/stderr buffering for unit tests

    * Minor: remove space left out by previous clean up

commit 3fcce51
Author: sauwming <ming@teluu.com>
Date:   Wed Feb 5 11:43:27 2025 +0800

    Fixed OpenSSL log error reading cert (#4291)

commit cbfbbc4
Author: jimying <yingqw.js@gmail.com>
Date:   Wed Feb 5 11:03:53 2025 +0800

    iocp: fix crash, GetQueuedCompletionStatus() write freed WSAOVERLAPPED memory (#4136)

commit 205baf0
Author: sauwming <ming@teluu.com>
Date:   Tue Feb 4 17:04:25 2025 +0800

    Fixed warnings in sip auth client (#4287)

commit abffe0d
Author: sauwming <ming@teluu.com>
Date:   Tue Feb 4 08:38:55 2025 +0800

    Fixed CI test failure (#4284)

commit 986fc78
Author: Johannes <johannes.westhuis@gmail.com>
Date:   Mon Feb 3 08:31:28 2025 +0100

    Share an auth session between multiple dialogs/regc (#4262)

commit 46111c4
Author: Nanang Izzuddin <nanang@teluu.com>
Date:   Mon Feb 3 11:36:54 2025 +0700

    Best effort avoid crash when media transport adapter not using group lock (#4281)

commit f986ad8
Author: Benny Prijono <bennylp@pjsip.org>
Date:   Fri Jan 31 09:45:19 2025 +0700

    Add link to coding style documentation (#4280)

commit 727ee32
Author: Nanang Izzuddin <nanang@teluu.com>
Date:   Fri Jan 31 09:08:02 2025 +0700

    Fix build error when PJ_LOG_MAX_LEVEL is zero (#4279)

    The `pj_log_get_log_func()` is not defined when PJ_LOG_MAX_LEVEL is set to zero.

    Thanks to Giorgio Alfarano for the report.

commit dae52f6
Author: Perry Ismangil <perry@teluu.com>
Date:   Thu Jan 30 08:43:54 2025 +0000

    Fixing typo (#4274)

    Acoustic

commit 1a4cd67
Author: sauwming <ming@teluu.com>
Date:   Thu Jan 30 15:21:49 2025 +0800

    Modify iOS sample apps dev team ID (#4278)

commit dfcfa13
Author: Tarteszeus <37761609+Tarteszeus@users.noreply.github.com>
Date:   Thu Jan 30 02:42:36 2025 +0100

    Add queried names to server address record, and add the address record in parameter for on_verify_cb callback (#4256)

commit f9e56d8
Author: Jan Tojnar <jtojnar@gmail.com>
Date:   Wed Jan 29 07:42:18 2025 +0100

    Fix duplicate function name in 100rel docs (#4275)

commit 960597e
Author: Nanang Izzuddin <nanang@teluu.com>
Date:   Wed Jan 29 13:36:34 2025 +0700

    Various works on SWIG Java (#4273)

    * Various works on SWIG Java

    1. Fix type mapping (SWIGTYPE_*):
       a. Map C "void*" & "void**" to Java long (was SWIGTYPE_p_void & SWIGTYPE_p_p_void which are not really usable), this should fix #4242.
       b. Map pjmedia_aud_dev_index to int.
       c. Map unsigned char[20] for SslCertInfo.serialNo to Java "short array"

       This also updates pjsua.i, e.g: tab->space, reorder things.

    2. Update swig_java_pjsua2.vcxproj:
       a. Rename config "Debug" & "Release" to "Debug-Dynamic" & "Release-Dynamic" in , as the project actually builds dynamic libs. Also fix the property sheet dependencies from *-static to *-dynamic.
       b. Update other settings, e.g: built tool version from 140 to 143.

    3. Update symbols.lst: added missing new types, tab->space, reorder alphabetically.

    * Update ci-win.yml
    * Add sample code for passing user data using utilTimerSchedule()
    * Add sample for cancelling timer

commit c36ed2c
Author: Benny Prijono <bennylp@pjsip.org>
Date:   Tue Jan 28 16:58:55 2025 +0700

    Minor modifications to Android build and samples to match new documentation (#4271)

    * To streamline the command, also clean swig and pjsua jni output directories when make distclean and realclean is called

    * Kotlin sample: add account, modify video size and bandwidth, and audio codec priorities to use AMR-WB

    * Android CLI app: fix armeabi hardcoded arch and also copy stdc++.so

commit bab33d6
Author: Noel Morgan <noel@vwci.com>
Date:   Tue Jan 28 01:51:50 2025 -0600

    Added support for updated RFC7866 content-type sub type with XML extension (#4270)

commit a89917e
Author: sauwming <ming@teluu.com>
Date:   Fri Jan 24 14:49:59 2025 +0800

    OpenSSL: Set ciphersuites only if not using BoringSSL (#4269)

commit 377a80c
Author: Nanang Izzuddin <nanang@teluu.com>
Date:   Fri Jan 24 13:48:31 2025 +0700

    Fix various compile errors & warnings in MSVC2005 (#4268)

commit de3f2e1
Author: sauwming <ming@teluu.com>
Date:   Fri Jan 24 10:20:28 2025 +0800

    Set CI vars in GH workflow file (#4263)

commit cdb1294
Author: sauwming <ming@teluu.com>
Date:   Thu Jan 23 11:01:27 2025 +0800

    Various fixes for Apple SSL backend (#4257)
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