[FFmpeg-devel] [PATCH] Reimplementation of OS/2 threads model

[Måns Rullgård]

KO Myung-Hun <komh at chollian.net> writes:

> Hi/2.
>
> This patch reimplements OS/2 threads model based on Win32 threads model.
>
> -- 
> KO Myung-Hun
>
> Using Mozilla SeaMonkey 2.0.3
> Under OS/2 Warp 4 for Korean with FixPak #15
> On AMD ThunderBird 1GHz with 512 MB RAM
>
> Korean OS/2 User Community : http://www.ecomstation.co.kr
>
>
> Index: libavcodec/os2thread.c
> ===================================================================
> --- libavcodec/os2thread.c	(revision 22960)
> +++ libavcodec/os2thread.c	(working copy)
> @@ -1,5 +1,5 @@
>  /*
> - * Copyright (c) 2004 Michael Niedermayer <michaelni at gmx.at>
> + * Copyright (c) 2010 KO Myung-Hun <komh at chollian.net>
>   *
>   * This file is part of FFmpeg.
>   *
> @@ -17,51 +17,120 @@
>   * License along with FFmpeg; if not, write to the Free Software
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
> -//#define DEBUG
>
> -// Ported by Vlad Stelmahovsky
> +#define INCL_DOS
> +#include <os2.h>
>
> +#undef __STRICT_ANSI__  /* for _beginthread() */
> +#include <stdlib.h>
> +
>  #include "avcodec.h"
>
> -#define INCL_DOS
> -#define INCL_DOSERRORS
> -#define INCL_DOSDEVIOCTL
> -#include <os2.h>
> +typedef struct Semaphore{
> +    HEV   hev;
> +    HMTX  hmtxWait;
> +    HMTX  hmtxCount;
> +    LONG  lCount;
> +    LONG  lMax;
> +}Semaphore;

Can we please not use Hungarian names?  Also, please add a space
before { and after }.  This applies to the entire patch.

>  typedef struct ThreadContext{
>      AVCodecContext *avctx;
> -    int thread;
> -    HEV work_sem;
> -    HEV done_sem;
> +    TID thread;
> +    Semaphore *work_sem;
> +    Semaphore *job_sem;
> +    Semaphore *done_sem;
>      int (*func)(AVCodecContext *c, void *arg);
> +    int (*func2)(AVCodecContext *c, void *arg, int, int);
>      void *arg;
> -    int ret;
> +    int argsize;
> +    int *jobnr;
> +    int *ret;
> +    int threadnr;
>  }ThreadContext;
>
> +#define INFINITE  ((ULONG)SEM_INDEFINITE_WAIT)

What's with the cast?

> +static Semaphore *CreateSemaphore(void *v1, LONG lCount, LONG lMax, void *v2){
> +    Semaphore *sem;
> +
> +    sem = av_mallocz(sizeof(Semaphore));
> +    if (!sem)
> +        return NULL;
> +
> +    DosCreateEventSem(NULL, &sem->hev, 0, lCount > 0 ? TRUE : FALSE );
> +    DosCreateMutexSem(NULL, &sem->hmtxWait, 0, FALSE );
> +    DosCreateMutexSem(NULL, &sem->hmtxCount, 0, FALSE );
> +
> +    sem->lCount = lCount;
> +    sem->lMax   = lMax;
> +
> +    return sem;
> +}
> +
> +static void ReleaseSemaphore(Semaphore *sem, LONG lInc, PLONG plPrevCount){
> +    DosRequestMutexSem(sem->hmtxCount, SEM_INDEFINITE_WAIT);
> +
> +    if (sem->lCount + lInc <= sem->lMax){
> +        sem->lCount += lInc;
> +        DosPostEventSem(sem->hev);
> +    }
> +
> +    DosReleaseMutexSem(sem->hmtxCount);
> +}
> +
> +static void WaitSemaphore(Semaphore *sem, ULONG ulTimeout){
> +    DosRequestMutexSem(sem->hmtxWait, SEM_INDEFINITE_WAIT);
> +
> +    DosWaitEventSem(sem->hev, ulTimeout);
> +
> +    DosRequestMutexSem(sem->hmtxCount, SEM_INDEFINITE_WAIT);
> +
> +    sem->lCount--;
> +    if (sem->lCount == 0){
> +        ULONG ulCount;
> +
> +        DosResetEventSem(sem->hev, &ulCount);
> +    }
> +
> +    DosReleaseMutexSem(sem->hmtxCount);
> +
> +    DosReleaseMutexSem(sem->hmtxWait);
> +}
> +
> +static void CloseSemaphore(Semaphore *sem){
> +    DosCloseEventSem(sem->hev);
> +    DosCloseMutexSem(sem->hmtxWait);
> +    DosCloseMutexSem(sem->hmtxCount);
> +
> +    av_freep(&sem);
> +}

Is this emulating something?  What's wrong with semaphore calls in the
old code?

>  static void attribute_align_arg thread_func(void *v){
>      ThreadContext *c= v;
>
>      for(;;){
> -        //printf("thread_func %X enter wait\n", (int)v); fflush(stdout);
> -        DosWaitEventSem(c->work_sem, SEM_INDEFINITE_WAIT);
> -//        WaitForSingleObject(c->work_sem, INFINITE);
> -//printf("thread_func %X after wait (func=%X)\n", (int)v, (int)c->func); fflush(stdout);
> +        int ret, jobnr;
> +        WaitSemaphore(c->work_sem, INFINITE);
> +        // avoid trying to access jobnr if we should quit
> +        if (!c->func && !c->func2)
> +            break;
> +        WaitSemaphore(c->job_sem, INFINITE);
> +        jobnr = (*c->jobnr)++;
> +        ReleaseSemaphore(c->job_sem, 1, 0);
>          if(c->func)
> -            c->ret= c->func(c->avctx, c->arg);
> +            ret= c->func(c->avctx, (uint8_t *)c->arg + jobnr*c->argsize);
>          else
> -            return;
> -        //printf("thread_func %X signal complete\n", (int)v); fflush(stdout);
> -        DosPostEventSem(c->done_sem);
> -//        ReleaseSemaphore(c->done_sem, 1, 0);
> +            ret= c->func2(c->avctx, c->arg, jobnr, c->threadnr);
> +        if (c->ret)
> +            c->ret[jobnr] = ret;
> +        ReleaseSemaphore(c->done_sem, 1, 0);
>      }
> -
> -    return;
>  }
>
>  /**
> - * free what has been allocated by avcodec_thread_init().
> - * must be called after decoding has finished, especially do not call while avcodec_thread_execute() is running
> + * Free what has been allocated by avcodec_thread_init().
> + * Must be called after decoding has finished, especially do not call while avcodec_thread_execute() is running.
>   */

Please do spelling fixes like this separately.

-- 
M?ns Rullg?rd
mans at mansr.com